Skip to content
  • Emmet O'Neill's avatar
    Refactor: Unified Color Picker Code & Removed Some Duplication · af5acee0
    Emmet O'Neill authored and Dmitry Kazakov's avatar Dmitry Kazakov committed
    Summary:
    This patch is part of task T8708, focused on further improving Krita's color picker.
    
    >As of right now, Krita effectively has two near-duplicate paths when it comes to color picking code: the code for the dedicated Color Selector Tool (P) exists mainly in "kis_tool_colorpicker", while the code for the ctrl-key activated color picker that you can pull up while other tools are active [as well as the scratchpad's picker] exists mainly in "kis_tool_utils".
    
    This patch addresses this issue by, as much as possible without some much bigger changes, unifying all of the core color picking functionality within the `KisToolUtils` namespace's `pickColor` function.
    
    Essentially all of the duplicate code was removed from `KisToolColorPicker::pickColor`, which now calls into the same `KisToolUtils::pickColor` function that's used elsewhere creating a single place in the code where things like sampling radius and color picker blending are implemented. As a result, any change to the code in `KisToolUtils::pickColor` will now affect all of Krita's color pickers (the dedicated tool [P], the ctrl-activated picker, and the scratchpad picker) in a uniform and predictable way.
    
    While this patch does solve the most of the code-duplication issue that existed in Krita's color pickers, there are still a few ways in which the dedicated tool and the ctrl-picker behave inconsistently. In my view, more extensive refactoring can be done in the future to create more uniform and predictable behavior across the various pickers, resulting in more color picking code being moved into `KisToolUtils::pickColor` from elsewhere. For now though, I wanted to focus on code-reuse improvements that don't have any user-facing design implications.
    
    Also, some minor code-style cleanup was done here and there.
    
    Test Plan:
    Test the various color pickers (dedicated tool, ctrl-picker, scratch pad) for *mostly predictable and equivalent behavior.
    
    *Certain aspects of the dedicated/ctrl picker can't be unified without some bigger changes, such as 'sample from merged/active only'.
    
    Reviewers: #krita, #krita_abyss, rempt, dkazakov
    
    Reviewed By: #krita, #krita_abyss, dkazakov
    
    Subscribers: #krita_abyss, #krita
    
    Tags: #krita_abyss
    
    Differential Revision: https://phabricator.kde.org/D12941
    af5acee0