Skip to content

Conversation

@HiCamino
Copy link
Contributor

this seemed like a pretty easy fix and works in my testing

but one weird thing is if you choose 'sample all layers' then it can get the color of the checkerboard bakground but i don't think that is fixable because of how the image is rendered in Artpaint?

Fixes #487

@humdingerb
Copy link
Member

A nice new feature! However. :)
As is - with the checkers being ignored when dealing with one layer, and included when sampling all layers - this difference in behaviour isn't acceptable IMO. Generally, the checker background must always be ignored, I think. Esp. when using a larger "Size" for the colour picker, sampling the checker background together with actual pixels of the image leads to wrongly calculated colours.

Isn't possible to add another function ReturnRenderedUncheckeredImage() to Image.cpp that does all the usual rendering, but without the BitmapUtilities::CheckerBitmap()?

@HiCamino
Copy link
Contributor Author

HiCamino commented Oct 19, 2025

i agree thats why i mentioned it. but its not that simple to fix because the ReturnRenderedImage just returns a pointer to the already rendered image and it has a checker bg already. I dont know why the renderedImage has a checker bg in the first place so i think that is what needs to be fixed but the image code is a bit complicated. it feels like you would never want a checker bg rendered in the image but i dont know yet how to make it just a bg that isnt in the image.

looks like when you save a image ArtPaint redraws with no bg and then saves it and redraws with the checker afterward. maybe making a copy of the image with no bg would work but it is a waste of memory

@HiCamino
Copy link
Contributor Author

i have ideas to fix it but maybe it should be a separate pr

@humdingerb
Copy link
Member

I dont know why the renderedImage has a checker bg in the first place

I'm not 100% sure, but at least one reason to have the checkers in there, is to keep the checkers fixed when zooming in/out etc. At least I think I remember something like that... Dunno if @dsizzle has time/recollection to chime in here.

i have ideas to fix it but maybe it should be a separate pr

Yeah, let's merge this one after the needed changes of another PR are merged.
In any case, as your solution will most probably be above my paygrade, make sure to elaborate in your commit messages exactly the hows and whys. Maybe also split into smaller commits if the changes are more substantial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Color picker: Add option to pick from all visible layers

2 participants