Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NOSQUASH] Editor: Replace the OKLab color picker by a 2D color picker #2895

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HybridDog
Copy link
Contributor

Goal

There are reports that the one-dimensional OKLab color selection is unpopular and difficult to use (see this Discussion),
so we can remove it to simplify the code and GUI.

A two-dimensional image can enable a more convenient selection of colors than a one-dimensional slider.
Under the assumption that users typically want to select a color with the highest saturation,
a two-dimensional color picker where any RGB color with the highest saturation can be slected with one click could be suitable.
For an independent adjustment of lightness and hue, which is considered a good property of a color picker,
the two-dimensional color picker should use a perceptual color space such as OKLab.
As a side effect for choosing a perceptual color space,
a marker showing the lightness and hue of the current color can visualize how a change in RGB values affects the perceived color.

Proposed changes

  • For the color selection in the editor, add the ItemColorPicker2D menu item,
    which displays an image and enables picking a color from it by clicking.
    The image contains the highest-saturation sRGB colors,
    so for the color picker we only need OKLab code to determine marker positions
    and no color clipping code since we can sample pixels from the image.
  • Add a method drawing a hexagon to Canvas
  • Add a helper method to ColorOKLCh which calculates a modified lightness
  • Add a missing include guard and constness to the code for ColorOKLCh
  • Remove ItemColorChannelOKLab and most of the OKLab color space code.
    Since the two-dimensional color picker uses some of the OKLab code,
    it is not removed completely.

Additional information

The author of OKLab has a webpage about a color picker, which uses a modified lightness.
I have also used the modified lightness since it leads to smaller dark regions in the color picker image and in SuperTux it is, I assume, uncommon to select dark colors (correct me if I'm wrong).

Color picker image with unmodified lightness:
out_L

Color picker image with modified lightness:
out_Lr

The new color picker with default language and 800x600 window size:
2024-04-06-174514_335x540_scrot

The color picker stretches horizontally; since the language affects the width of the menu title, the color picker's aspect ratio depends on the language. I don't think this is a problem, so I haven't implemented it differently.

I couldn't find a way to get a pixel from a Surface, so the color picker image is loaded twice: one time for the drawing and one time to sample pixel values from it.

Please test the new color picker.

@tobbi
Copy link
Member

tobbi commented Apr 6, 2024

There's various compiler errors:

D:\a\supertux\supertux\src\video\canvas.cpp(340,21): error C2641: cannot deduce template arguments for 'std::array' [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
D:\a\supertux\supertux\src\video\canvas.cpp(340,21): error C2780: 'std::array<_Ty,_Size> std::array(void)': expects 0 arguments - 6 provided [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
  C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include\utility(147,7):
  see declaration of 'std::array'
  
D:\a\supertux\supertux\src\video\canvas.cpp(340,21): error C2780: 'std::array<_Ty,_Size> std::array(std::array<_Ty,_Size>)': expects 1 arguments - 6 provided [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
  C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include\utility(147,7):
  see declaration of 'std::array'
  
D:\a\supertux\supertux\src\video\canvas.cpp(348,26): error C2027: use of undefined type 'std::array' [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
  C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include\utility(147,7):
  see declaration of 'std::array'
  
D:\a\supertux\supertux\src\video\canvas.cpp(349,34): error C2676: binary '[': 'std::array' does not define this operator or a conversion to a type acceptable to the predefined operator [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
D:\a\supertux\supertux\src\video\canvas.cpp(349,56): error C2676: binary '[': 'std::array' does not define this operator or a conversion to a type acceptable to the predefined operator [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
D:\a\supertux\supertux\src\video\canvas.cpp(349,78): error C2676: binary '[': 'std::array' does not define this operator or a conversion to a type acceptable to the predefined operator [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]
D:\a\supertux\supertux\src\video\canvas.cpp(349,5): error C2660: 'Canvas::draw_triangle': function does not take 3 arguments [D:\a\supertux\supertux\build\supertux2_lib.vcxproj]

@MatusGuy MatusGuy added category:code status:needs-review Work needs to be reviewed by other people labels Apr 7, 2024
A two-dimensional image can enable a more convenient selection of colors than a one-dimensional slider.
Under the assumption that users typically want to select a color with the highest saturation,
a two-dimensional color picker where any RGB color with the highest saturation can be slected with one click could be suitable.
For an independent adjustment of lightness and hue, which is considered a good property of a color picker,
the two-dimensional color picker should use a perceptual color space such as OKLab.
As a side effect for choosing a perceptual color space,
a marker showing the lightness and hue of the current color can visualize how a change in RGB values affects the perceived color.

Changes:
* For the color selection in the editor, add the ItemColorPicker2D menu item,
  which displays an image and enables picking a color from it by clicking.
  The image contains the highest-saturation sRGB colors,
  so for the color picker we only need OKLab code to determine marker positions
  and no color clipping code since we can sample pixels from the image.
* Add a method drawing a hexagon to Canvas
* Add a helper method to ColorOKLCh which calculates a modified lightness
* Add a missing include guard and constness to the code for ColorOKLCh
There are reports that the one-dimensional OKLab color selection is unpopular and difficult to use,
so we can remove it to simplify the code and GUI.

This commit removes ItemColorChannelOKLab and most of the OKLab color space code.
Since the two-dimensional color picker uses some of the OKLab code,
it is not removed completely.
@HybridDog
Copy link
Contributor Author

I've fixed the compiler errors; the MacOS checks also fail in the master branch.

@tobbi
Copy link
Member

tobbi commented Apr 12, 2024

Looks good, I guess. The drawing methods I didn't look at in detail because urgh, code but from what I can tell this is good to merge.

@tobbi
Copy link
Member

tobbi commented Apr 19, 2024

This good to merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:code involves:editor status:needs-review Work needs to be reviewed by other people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants