Skip to content

feat: add screen magnifier toggle#1116

Merged
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master
May 20, 2026
Merged

feat: add screen magnifier toggle#1116
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602
Copy link
Copy Markdown
Contributor

@fly602 fly602 commented May 20, 2026

  1. Add a new dsettings key viewZoomEnable to control whether screen magnifier shortcuts are enabled.
  2. Initialize and store a dedicated keybinding config manager so the shortcut manager can read the new keybinding setting at runtime.
  3. Apply the magnifier switch during startup by calling SetViewZoomEnabled, ensuring the zoom feature state matches persisted configuration immediately after initialization.
  4. Listen for viewZoomEnable changes in dconfig and dynamically enable or disable the related zoom shortcuts without requiring a restart.
  5. Introduce dedicated handling for zoom-related KWin shortcut IDs (viewZoomIn, viewZoomOut, viewActualSize), including adding them only when enabled and removing them when disabled.
  6. Prevent zoom shortcuts from being added through the normal KWin shortcut loading flow to avoid duplicate registration and shortcut invalidation during initialization.
  7. Control the KWin zoom effect through gdbus by checking whether the effect is loaded and loading or unloading it based on the configured switch.
  8. Use safe fallback behavior by treating the feature as enabled when configuration cannot be read, which helps preserve existing behavior and avoid accidentally disabling magnifier functionality.

Log: Added a screen magnifier toggle to dynamically enable or disable zoom shortcuts and the zoom effect

Influence:

  1. Verify that when viewZoomEnable is true, viewZoomIn, viewZoomOut, and viewActualSize shortcuts are available and trigger the expected magnifier behavior.
  2. Verify that when viewZoomEnable is false, the related zoom shortcuts are removed and no longer take effect.
  3. Test startup behavior with the switch enabled and disabled to confirm the runtime state matches the saved configuration after login or daemon restart.
  4. Toggle the setting at runtime and confirm the shortcuts are added or removed immediately without restarting the session.
  5. On KWin environments, verify that the zoom effect is loaded when enabled and unloaded when disabled.
  6. Test fallback behavior when dsettings or gdbus access fails, ensuring the daemon does not crash and existing shortcut behavior remains as safe as possible.
  7. Verify there is no duplicate shortcut registration or shortcut loss for zoom-related actions during initialization on both X11 and Wayland paths.

feat: 添加屏幕放大镜开关

  1. 新增 dsettings 配置项 viewZoomEnable,用于控制是否启用屏幕放大镜相 关快捷键。
  2. 初始化并保存独立的 keybinding 配置管理器,使快捷键管理器能够在运行时 读取该按键绑定配置。
  3. 在启动初始化阶段调用 SetViewZoomEnabled 应用放大镜开关,确保启动后 缩放功能状态与持久化配置保持一致。
  4. 在 dconfig 变更监听中增加对 viewZoomEnable 的处理,使相关缩放快捷键 可以在运行时动态启用或禁用,无需重启。
  5. 为 KWin 的缩放相关快捷键 ID(viewZoomInviewZoomOutviewActualSize)增加专门处理逻辑,仅在开关开启时添加,关闭时删除。
  6. 在常规 KWin 快捷键加载流程中跳过缩放快捷键,避免初始化阶段重复注册导 致快捷键失效。
  7. 通过 gdbus 控制 KWin 的 zoom 特效,先检查特效加载状态,再根据开关 决定加载或卸载该特效。
  8. 对配置读取失败场景采用默认启用的兜底策略,以保持现有行为不变,避免误 关闭放大镜功能。

Log: 新增屏幕放大镜开关,可动态启用或禁用缩放快捷键及缩放特效

Influence:

  1. 验证当 viewZoomEnabletrue 时,viewZoomInviewZoomOutviewActualSize 快捷键可用,并能触发预期的放大镜行为。
  2. 验证当 viewZoomEnablefalse 时,相关缩放快捷键会被移除,且不再 生效。
  3. 测试在开关开启和关闭两种情况下的启动行为,确认登录或守护进程重启后运 行时状态与保存配置一致。
  4. 在运行时切换该设置,确认无需重启会话即可立即添加或移除对应快捷键。
  5. 在 KWin 环境下验证开启时会加载 zoom 特效,关闭时会卸载该特效。
  6. 测试 dsettings 或 gdbus 访问失败时的兜底行为,确保守护进程不会崩 溃,并尽可能保持原有快捷键行为稳定。
  7. 验证在 X11 和 Wayland 路径下初始化过程中不会出现缩放快捷键重复注册或 丢失的问题。

PMS: BUG-360027
Change-Id: Ic9d9617178a8bec0427205f18b11d89c97d24d14

1. Add a new dsettings key `viewZoomEnable` to control whether screen
magnifier shortcuts are enabled.
2. Initialize and store a dedicated keybinding config manager so the
shortcut manager can read the new keybinding setting at runtime.
3. Apply the magnifier switch during startup by calling
`SetViewZoomEnabled`, ensuring the zoom feature state matches persisted
configuration immediately after initialization.
4. Listen for `viewZoomEnable` changes in dconfig and dynamically enable
or disable the related zoom shortcuts without requiring a restart.
5. Introduce dedicated handling for zoom-related KWin shortcut IDs
(`viewZoomIn`, `viewZoomOut`, `viewActualSize`), including adding them
only when enabled and removing them when disabled.
6. Prevent zoom shortcuts from being added through the normal KWin
shortcut loading flow to avoid duplicate registration and shortcut
invalidation during initialization.
7. Control the KWin `zoom` effect through `gdbus` by checking whether
the effect is loaded and loading or unloading it based on the configured
switch.
8. Use safe fallback behavior by treating the feature as enabled when
configuration cannot be read, which helps preserve existing behavior and
avoid accidentally disabling magnifier functionality.

Log: Added a screen magnifier toggle to dynamically enable or disable
zoom shortcuts and the zoom effect

Influence:
1. Verify that when `viewZoomEnable` is `true`, `viewZoomIn`,
`viewZoomOut`, and `viewActualSize` shortcuts are available and trigger
the expected magnifier behavior.
2. Verify that when `viewZoomEnable` is `false`, the related zoom
shortcuts are removed and no longer take effect.
3. Test startup behavior with the switch enabled and disabled to confirm
the runtime state matches the saved configuration after login or daemon
restart.
4. Toggle the setting at runtime and confirm the shortcuts are added or
removed immediately without restarting the session.
5. On KWin environments, verify that the `zoom` effect is loaded when
enabled and unloaded when disabled.
6. Test fallback behavior when dsettings or `gdbus` access fails,
ensuring the daemon does not crash and existing shortcut behavior
remains as safe as possible.
7. Verify there is no duplicate shortcut registration or shortcut loss
for zoom-related actions during initialization on both X11 and Wayland
paths.

feat: 添加屏幕放大镜开关

1. 新增 dsettings 配置项 `viewZoomEnable`,用于控制是否启用屏幕放大镜相
关快捷键。
2. 初始化并保存独立的 keybinding 配置管理器,使快捷键管理器能够在运行时
读取该按键绑定配置。
3. 在启动初始化阶段调用 `SetViewZoomEnabled` 应用放大镜开关,确保启动后
缩放功能状态与持久化配置保持一致。
4. 在 dconfig 变更监听中增加对 `viewZoomEnable` 的处理,使相关缩放快捷键
可以在运行时动态启用或禁用,无需重启。
5. 为 KWin 的缩放相关快捷键 ID(`viewZoomIn`、`viewZoomOut`、
`viewActualSize`)增加专门处理逻辑,仅在开关开启时添加,关闭时删除。
6. 在常规 KWin 快捷键加载流程中跳过缩放快捷键,避免初始化阶段重复注册导
致快捷键失效。
7. 通过 `gdbus` 控制 KWin 的 `zoom` 特效,先检查特效加载状态,再根据开关
决定加载或卸载该特效。
8. 对配置读取失败场景采用默认启用的兜底策略,以保持现有行为不变,避免误
关闭放大镜功能。

Log: 新增屏幕放大镜开关,可动态启用或禁用缩放快捷键及缩放特效

Influence:
1. 验证当 `viewZoomEnable` 为 `true` 时,`viewZoomIn`、`viewZoomOut`、
`viewActualSize` 快捷键可用,并能触发预期的放大镜行为。
2. 验证当 `viewZoomEnable` 为 `false` 时,相关缩放快捷键会被移除,且不再
生效。
3. 测试在开关开启和关闭两种情况下的启动行为,确认登录或守护进程重启后运
行时状态与保存配置一致。
4. 在运行时切换该设置,确认无需重启会话即可立即添加或移除对应快捷键。
5. 在 KWin 环境下验证开启时会加载 `zoom` 特效,关闭时会卸载该特效。
6. 测试 dsettings 或 `gdbus` 访问失败时的兜底行为,确保守护进程不会崩
溃,并尽可能保持原有快捷键行为稳定。
7. 验证在 X11 和 Wayland 路径下初始化过程中不会出现缩放快捷键重复注册或
丢失的问题。

PMS: BUG-360027
Change-Id: Ic9d9617178a8bec0427205f18b11d89c97d24d14
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @fly602, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。本次代码变更主要为KWin的缩放快捷键增加了一个可通过DConfig动态控制的启用/禁用开关功能。

整体来看,代码逻辑清晰,功能划分明确。但在语法逻辑、代码质量、性能和安全性方面,有一些值得注意和改进的地方。以下是详细的审查意见:

1. 语法与逻辑

  • 类型断言缺乏安全检查 (Panic 风险)
    IsViewZoomEnabled 函数中:

    return v.Value().(bool)

    如果 DConfig 配置文件被手动篡改,或者类型不一致,直接使用 .(bool) 会在运行时引发 panic,导致整个守护进程崩溃。
    改进建议:使用带有 ok 的安全类型断言,或者在断言失败时返回默认值 true

    val := v.Value()
    if b, ok := val.(bool); ok {
        return b
    }
    logger.Warningf("invalid type for config %s: %T", constants.DSettingsKeyViewZoomEnable, val)
    return true // 默认开启
  • initDconfig 中的提前返回导致初始化中断
    shortcut_manager.goinitDconfig 方法中:

    if err != nil || keybindingConfigPath == "" {
        logger.Warning(err)
        return // <--- 这里直接 return 了
    }

    如果获取 keybindingConfigPath 失败,函数直接 return,这会导致后续如果还有其他 DConfig 初始化逻辑(虽然当前 diff 末尾没有,但未来可能会加)被跳过。更重要的是,sm.keybindingConfigMgr 将保持为 nil
    改进建议:移除 return,或者仅跳过当前配置管理器的初始化,让程序继续执行。

  • JSON 文件末尾缺少换行符
    Diff 显示 org.deepin.dde.daemon.keybinding.json 文件末尾 \ No newline at end of file。在 POSIX 标准中,文本文件应以换行符结尾,部分工具(如 Git、CMake)可能会对此发出警告。
    改进建议:在 JSON 文件最后添加一个空行。

2. 代码质量

  • 硬编码的 D-Bus 目标和路径
    SetViewZoomEnabled 中硬编码了 "org.kde.KWin""/Effects"

    return []string{"call", "--session", "--dest", "org.kde.KWin", "--object-path", "/Effects", ...}

    如果 KWin 升级修改了路径,或者需要适配 Wayland 下的其它窗口管理器,维护起来较困难。
    改进建议:建议将这些常量提取到文件顶部或专门的常量文件中,并增加注释说明为何使用 KWin 的此接口。

  • viewZoomIds 数据结构选择
    var viewZoomIds = map[string]bool 使用 map[string]bool 来表达集合在 Go 中是常见的,但 bool 占用 1 字节。更符合 Go 惯例且零内存占用的写法是使用空结构体。
    改进建议

    var viewZoomIds = map[string]struct{}{
        "viewZoomIn":     {},
        "viewZoomOut":    {},
        "viewActualSize": {},
    }
  • 重复的注释
    AddKWinAddKWinForWayland 中有完全相同的注释:

    // zoom功能需要根据配置决定是否添加, 这里先不添加,避免在初始化配置时重复设置,导致快捷键失效

    改进建议:可以稍微精简,比如:// 缩放快捷键由 viewZoomEnable 配置单独控制,此处跳过

3. 代码性能

  • 频繁调用外部进程 (gdbus)
    SetViewZoomEnabled 每次调用都会通过 exec.Command("gdbus", ...) 执行外部命令。进程的创建和销毁开销较大。如果在短时间内频繁切换配置,会造成性能抖动。
    改进建议

    1. 防抖:在 Manager 层面对 SetViewZoomEnabled 的调用增加防抖逻辑,避免短时间内多次触发。
    2. 原生 D-Bus 调用:既然项目已经引入了 godbus,建议直接使用 Go 的 D-Bus 库进行方法调用,而不是 fork 子进程调用 gdbus,这能将耗时从毫秒级降低到微秒级。
  • strings.ToLower 的不必要内存分配

    loadzoom := strings.Contains(strings.ToLower(string(output)), "true")

    output[]byte,转成 stringToLower 会产生两次内存分配。因为布尔值只有 true/false,且通常 D-Bus 返回的是小写。
    改进建议:直接使用 bytes.Contains,或者精确匹配:

    loadzoom := bytes.Contains(bytes.ToLower(output), []byte("true"))

4. 代码安全

  • 命令注入风险 (低风险)
    exec.Command("gdbus", gdbusArgs(method)...) 中的参数目前都是硬编码的字符串常量,没有来自用户的直接输入,因此当前不存在命令注入风险
    但是,使用 exec.Command 依然属于高风险模式的代码。未来如果有人重构这部分代码,将 methodzoom 暴露为变量,就可能引发注入。
    改进建议:如前所述,替换为 Go 原生 D-Bus 调用,从根本上杜绝命令注入的可能性,同时提升性能。

综合改进后的代码示例

针对上述意见,以下是修改后的核心代码片段参考:

shortcut_manager.go

var viewZoomIds = map[string]struct{}{
	"viewZoomIn":     {},
	"viewZoomOut":    {},
	"viewActualSize": {},
}

// KWin D-Bus 相关常量
const (
	kWinDest       = "org.kde.KWin"
	kWinEffectsPath = "/Effects"
	kWinEffectsIface = "org.kde.kwin.Effects"
)

func (sm *ShortcutManager) IsViewZoomEnabled() bool {
	if sm.keybindingConfigMgr == nil {
		return true
	}
	v, err := sm.keybindingConfigMgr.Value(0, constants.DSettingsKeyViewZoomEnable)
	if err != nil {
		return true
	}
	// 安全的类型断言
	val := v.Value()
	if b, ok := val.(bool); ok {
		return b
	}
	logger.Warningf("invalid type for config %s: %T", constants.DSettingsKeyViewZoomEnable, val)
	return true
}

func (sm *ShortcutManager) DelViewZoomShortcuts() {
	for id := range viewZoomIds {
		shortcut := sm.GetByIdType(id, ShortcutTypeWM)
		if shortcut != nil {
			sm.Delete(shortcut)
		}
	}
}

func (sm *ShortcutManager) SetViewZoomEnabled(wmObj wm.Wm, enabled bool) {
	logger.Debugf("SetViewZoomEnabled: %v", enabled)

	gdbusArgs := func(method string) []string {
		return []string{"call", "--session", "--dest", kWinDest,
			"--object-path", kWinEffectsPath,
			"--method", kWinEffectsIface + "." + method, "zoom"}
	}

	runGdbus := func(method string) {
		if _, err := exec.Command("gdbus", gdbusArgs(method)...).Output(); err != nil {
			logger.Warningf("Failed to %s zoom effect: %v", method, err)
		} else {
			logger.Infof("Successfully %s zoom effect", method)
		}
	}

	output, err := exec.Command("gdbus", gdbusArgs("isEffectLoaded")...).Output()
	if err != nil {
		sm.DelViewZoomShortcuts()
		logger.Warning(err)
		return
	}
	
	// 优化字节处理,减少内存分配
	loadzoom := bytes.Contains(bytes.ToLower(output), []byte("true"))

	if enabled {
		sm.addViewZoomKWin(wmObj)
		if !loadzoom {
			runGdbus("loadEffect")
		}
	} else {
		sm.DelViewZoomShortcuts()
		if loadzoom {
			runGdbus("unloadEffect")
		}
	}
}

// initDconfig 修改建议
func (sm *ShortcutManager) initDconfig() {
	// ... 原有逻辑 ...

	keybindingConfigPath, err := ds.AcquireManager(0, constants.DSettingsAppID, constants.DSettingsKeyBindingName, "")
	if err != nil || keybindingConfigPath == "" {
		logger.Warning(err)
		// 不要 return,避免阻断后续可能的初始化逻辑
	} else {
		sm.keybindingConfigMgr, err = configManager.NewManager(bus, keybindingConfigPath)
		if err != nil {
			logger.Warning(err)
		}
	}
}

希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时提问。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fly602 fly602 merged commit b156077 into linuxdeepin:master May 20, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants