Hi Lulu!

Thanks for the draft. It is a good base for implementation. It's apparent that a lot of consideration went into addressing various use-case scenarios, which is commendable. Let's dive into the details of it.

The FChooseColorSimpleRedraw naming pattern (with Fprefix) is reserved for private fields. Perhaps renaming it to NotifyColorBinding would be more apt and also allow for future expansions if needed.

In a similar fashion, I suggest to rename GetDigitFromColorsBindToKey to GetKeyAssociatedToColor. For now it is a digit, but it could be something else.

Forwarding keys to the main form is a wise move. It would be beneficial to also relay the key press events (using the UTF8KeyPress event) to ensure a consistent behavior throughout.

The '6' key conflict is a natural outcome considering the French keyboard layouts. Other layouts will have similar conflicts. I'm inclined towards leaving this as it is for now. The user can hold Shift or not using this digit to work around this.

About the effect of pressing digits changing the alpha slider, I've found the reason and fixed it on dev. It was because the opacity updown in the pen color interface was inadvertently active. I disabled it until it is visible so it won't change the alpha value. I invite you to retrieve last version. You can proceed with re-enabling the numpad check.

You say that when the user recall a color with alpha different than 255, the right alpha is set only if the palette is in mode includes alpha channel. This is intriguing indeed. If we keep the idea of following the palette option, which will make sense if we add an option to save the palette with the key bindings, I propose the following.

If the palette is not in alpha mode:

• When adding a color, then register it with alpha opaque (255).
• When looking up for a color to see its association, ignore the alpha channel. This ensures that any opacity alterations in the color chooser won't incidentally remove the digit indicator.

Some additional observations on my end:

• When binding a color to a digit and then to a another digit, the color is mapped to both keys. I suggest to unregister the other digit by looping over the registered colors. For instance, if initially bound to '2' and then '5', it's a bit unexpected to see it still mapped to '2' in the color chooser.
• Strangely, the digit doesn't seem to display in the palette for me. When I attempted to trace it back, it appears the color wasn't recognized among the registered ones. Have you encountered a similar behavior on your end?

Don't hesitate to come back to me to do more testing. Warm regards

Hi, I've done the changes you've suggested.

About ignore alpha channel when looking up for a color to see its association (palette not in alpha mode), it introduce a new issue: at startup, the black color in the palette is displayed with the 0 digit. I refactored the code to avoid this.

About the digit doesn't seem to display in the palette: do you added the color to the palette ?

• Here if I pick a color from the palette then press Ctrl 2 the digit 2 is displayed in the color cell in the palette and in the choose color window preview.

• If I pick a color from the current layer (with the color picker) then press Ctrl 5 the digit 5 is displayed in the choose color window, nothing change in the palette. Now if I click the Add color to palette button the new color is added to the palette and displayed with the digit 5 and the sign.

Assigning a color to a digit don't add it automatically to the palette: this behavior is ok ?

Edit: there is another keyboard conflict (at least on Windows): to reproduce, assign a color to a digit, choose the pen tool, change the pen width with the up/down arrow or with the slider or enter its value with the keyboard. Now press the digit to recall the registered color -> the pen width change. This is because the pen width TBCTrackBarUpDown still have the focus. Clicking to another part of the main window or press ENTER/ESC don't remove the focus on the edit.

Hi Lulu!

Firstly, thank you for the rapid iterations and your detailed observations. This allows me to have a clear view of the intricacies.

I understand the problem with transparent color being like black and the refactoring you applied. There are some cases that are not handled yet:

• the color choose has the number "0" displayed when the current color is black on startup because black is not differentiated from transparent.
• if the palette had the transparent color, it would have automatically a digit
• when retrieving a color one could want to retrieve transparent, even though that's rare

To solve all these issues, I propose the following:

• in the ColorMatch function, set the alpha to 255 only if the alpha is different from 0. This will treat the transparent color as a distinct one.
• the aForceCheckAlpha parameter is no longer necessary with the change above
• in the structure that holds the registered colors, handle an "undefined" value instead of transparent, in order to differentiate it from an actual "transparent" color

We could utilize the capabilities of fgl unit (stands for FreePascal Generic List) to handle undefined values, by using a map/dictionary instead of an array. Here is a potential setup, supposing we are still indexing with an integer:

uses fgl;

type
  TBGRAPixelBinding = specialize TFPGMap<integer, TBGRAPixel>; 

This type to hold the colors, needs to be created/freed as a regular object. To set a new binding you can still do FColorsBindToKey[AIndex] := AColor but to retrieve the color, you need to check for the existence of the index, for example using the TryGetData function that will return true if it succeeds.

Building on the notion of managing color bindings, perhaps introducing an option to unbind a color by employing a combo like Ctrl-Alt and a digit (0-9) could enhance user flexibility.

Assigning a color to a digit don't add it automatically to the palette: this behavior is ok ?

It seems reasonable at the moment. We can observe user feedback and make adjustments if needed.

As for the visibility issue, with latest version, I can see a digit in the palette, but it is very small. I suggest to set the height of the font explicitely, for example via FontFullHeight property of Bitmap.

Regarding the keyboard conflict, this is a tricky one. You're right, there are certain controls like the TBCTrackBarUpDown that retain focus. I've had a similar problem with the text tool where typing digits in the text would not be possible.

At the end of the TFMain.PictureMouseMove procedure, in LazpaintMainForm unit, there is a check to call Layout.FocusImage. It applies only to Text and EditShape tools for now and specific spin edits. I suggest to ignore the current tool and simply check for all spin edits instead (expanding existing function and changing its name). This way, moving the cursor above the image will automatically release the focus.

It would make sense as well to release the focus when one clicks on the Panel_ToolbarBackground.

The aim here is of course to make the experience seamless for the user. I'm confident that with these tweaks and constant communication between us, we can get there. So please keep the feedback coming!

Hi!
Understood the modifications to do. I've never use TFPGMap before, it looks very handy.

Building on the notion of managing color bindings, perhaps introducing an option to unbind a color by employing a combo like Ctrl-Alt and a digit (0-9) could enhance user flexibility.

What do you think if Ctrl+digit toggle the bindind ? Only one key to keep in memory, good for lazy user like me!

Hi !

I've never use TFPGMap before, it looks very handy.

Yes, maps/dictionaries are a fantastic structure.

What do you think if Ctrl+digit toggle the bindind ? Only one key to keep in memory, good for lazy user like me!

That's a good point! I like the simplicity. Let's go with that approach.

Hi, I was busy yesterday.

supposing we are still indexing with an integer

Would you suggest using a key based on UTF8String to be ready for a possible future extension using keys other than digits?

If the answer is yes, I suggest to keep the key detection in TPaletteToolbar.CatchToolKeyUp/Down and translate the VK_xxx to a string digit by code. Because if we add the function CatchToolKeyPress(var AKey: TUTF8Char): boolean; to TPaletteToolbar, the variable AKey does not reflect the value we are interested in. On my AZERTY keyboard, it gives à when I press 0 (not in the numpad). on QWERTY the value will certainly differ and so on.

Hello Lulu,

That's completely alright. Everyone has those busy days.

Would you suggest using a key based on UTF8String to be ready for a possible future extension using keys other than digits?

You're thinking ahead, which is great. I was rather mentioning it because when you define a map you can choose the key type. For now it is integer, so that's the type to provide to the map.

I suggest to keep the key detection in TPaletteToolbar.CatchToolKeyUp/Down and translate the VK_xxx to a string digit by code

Indeed, using the virtual key codes (VK_xxx) to discern the intent of the user can be more reliable than relying on the actual character produced. In theory, we need both KeyUp/Down and KeyPress to handle all possibilities but for now we don't need KeyPress. We can add it later on.

Your remarks are spot on. I suggest to focus for now on a simple version handling only digits, but it is always great to have some idea of where the code might be going.

Warm regards

Hi Circular, I move forward cautiously and in small steps.

At the end of the TFMain.PictureMouseMove procedure, in LazpaintMainForm unit, there is a check to call Layout.FocusImage. It applies only to Text and EditShape tools for now and specific spin edits. I suggest to ignore the current tool and simply check for all spin edits instead (expanding existing function and changing its name). This way, moving the cursor above the image will automatically release the focus.

If I understand correctly, the function TextSpinEditFocused could be renamed to SpinEditFocused and its code will check all the SpinEdit in the Main form.

Currently the TFMain.TextSpinEditFocused function is used in TFMain.FormKeyDown and TFMain.FormUTF8KeyPress. Can you confirm that the additions won't cause any problems here?

Hi Lulu! It's great your considering all details and possible side effects.

About the addition to TextSpinEditFocused function, yes, you're clear to do the change. The call to this function is in the case of the text tool, and in this case, only the spin edits relevant to text are visible and can be focused.

Hi Circular,
I've done the changes with (again!) some additions:

• refactored function SpineditFocused (previously TextSpinEditFocused) using Screen.ActiveControl to detect focus on any TBCTrackbarUpDown instance (also those hidden in TLCVectorialFillControl)
• added check in TFMain.CatchToolKeyDown(...) to avoid to recall a binded color when the keyboard digits are pressed while editing a text (Text tool) or editing a TBCTrackbarUpDown instance.

Don't hesitate if you see any possible bug, better code construction or better name for procedure!

Regards

Hi Lulu!

Thanks for the thorough job. We did it. I have merged the changes! One new feature for LazPaint :)

I've made a minor adjustment on the dev-lazpaint branch regarding the key event and the name of a method. Here it is if you are curious: 406cccb

Warm regards

Cool !
I couldn't have done it without your advice. It would take me several lifetimes to analyze the code of BGRABitmap/LazPaint and certainly a much better level in mathematics!

Thank you for giving me the opportunity to participate in your project! 👍

Hi Lulu!

I acknowledge the complexity of the codebase, it's been a journey spanning decades to write it all. Given this, I am considering ways to make it more modular, hoping it will simplify modifications and further improvements in the future.

It's a pleasure having you on board! Contributions like yours are what make open source projects thrive. Thank you for your kind words.

Warm regards