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
Image::blend_rect might not blend as expected #31124
Comments
Maybe an additional parameter could determine what to do with alpha. |
Sorry I should add a reproduction project because it really didn't work as expected for me. |
Run |
pos(62,2) shows # 132e3f A7 on expected.png (tested with aseprite) looks like the expected.png also has an error Which leads to this solution Line 2207 in 7e9c104
|
I think that difference is meaningless since that pixel is located in a transparent area. Godot saves PNGs without compressing the transparent areas, while my painting software does. It's not related to the issue, which is about the top-left corner. Are you suggesting we change the blending formula? |
Yes it looks like it is not working properly. |
I've tried the solution above, testing with pixelorama to see results, and indeed it works better. |
Photoshop style blending is a bit different to game style blending, as you need to deal with situations like blending onto a transparent background, whereas usual game blending is designed to be fast. There are also other tradeoffs with speed and accuracy. So if you want to support this you might be best off with a separate routine / code path for the photoshop style blending. Of course if you wanted to make photoshop like app you would probably need other blending modes too. And consider the colour spaces, doing your blending in linear for instance. In practice for anything except simple apps I think you'd need to write some custom code in c++ for such an app. |
So, this could be addressed adding a 4th param, something like a bool |
@azagaya Sounds good to me. Personally, I'd name the parameter |
@Calinou And should the same be implemented in |
@azagaya Probably, it makes sense to implement it there as well if it can be done easily enough. |
@Calinou Also, now that i'm there, if the source pixel is fully transparent, we could avoid blending the colors, as it should result in the destination color anyways... similar to how mask works in |
Myself I would like to advocate replacing the old behaviour completely. My reasoning being:
Overall it seems to be a very simple bug and until a proper rich blending library is maybe made someday, replacing the current behaviour with Color.blend() is the most sane and correct solution which adds less lines than it removes. |
That makes sense to me. If some core contributor is ok with that, i can replace the PR to do exactly what you say. |
I've tryed and seems to work perfectly fine with |
godotengine/godot#31124 has now been fixed in Godot 3.2.2-rc1, so we can use Image.blend_rect() instead of a custom method. This makes exporting large images and drawing with large brush sizes a lot faster. Once Godot 3.2.2 stable is released, the custom blend_rect method will be completely removed.
Godot 3.1.1
tl;dr:
Image::blend_rect()
may cause transparent regions to darken, contrary to what you would get with a drawing app.Explanation:
When trying to work on a painting app in which you can paint on transparent images using shaders and a transparent viewport, I got to deal with alpha blending. I tried blending a diagonal white-to-transparent-white gradient with the Godot icon, and quickly stumbled on the following problem:
Expected (Paint.NET):
Obtained (Godot):
Notice the darkened corner. From a pure math standpoint, it's just the result of averaging colors from the "invisible" pixels of the destination image, ending up darkening the gradient. But it may not be the expected result, at least to me it wasn't.
So I went bulldozer mode and blended myself in shaders, coming up with this:
That last division did the trick.
But then, looking at
Image::blend_rect
, I noticed it does the same thing, except the division. So I tested it, and it has the same issue.So is this really expected or should it be patched?
The text was updated successfully, but these errors were encountered: