Bug 356462 - Conversion from integer to float and back gives rounding errors for opacity
Summary: Conversion from integer to float and back gives rounding errors for opacity
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Color models (show other bugs)
Version: 2.9.10
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: Krita Bugs
URL: https://youtu.be/7BSk9WZPHBQ
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-10 12:33 UTC by barny.calhoun
Modified: 2024-11-18 15:28 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
File with 50% transparent black layer that doesn't go to 0 even after 10~ layers. (221.42 KB, application/x-krita)
2016-01-04 15:21 UTC, wolthera
Details

Note You need to log in before you can comment on or make changes to this bug.
Description barny.calhoun 2015-12-10 12:33:39 UTC
Krita offset color values when i change opacity

Reproducible: Always

Steps to Reproduce:
1. Change opacity to 50%
2. Draw until u get 100% opacity
3. Pick color 

Actual Results:  
Krita offset color values by 1. Happens with all colors.

Expected Results:  
Don't offset color values
Comment 1 wolthera 2015-12-10 15:41:02 UTC
Yeah, this one is caused by a rounding error:

0+1=1
1/2=0.5
int(0.5)=1.

So all mixes where the result befor dividing is uneven end up with an offset.

This doesn't happen with floating point. No clue what should be done about it...
Comment 2 wolthera 2015-12-10 15:47:44 UTC
Actually, no, now I think about it, the problem happens in the transparency code: Somehow we're not ending up with 100% transparency.

You can test this by comparing the difference between 'pick from merged image' and 'pick from layer'. On 'pick from layer' we get full black. On 'pick from merged image' integer spaces give an offset. So the error happens in the transparancy-mixing code, and that it is unable to get to 100%.
Comment 3 Sven Langkamp 2015-12-10 19:45:53 UTC
It works here, but it depends a bit on how often you paint over the same place. With 50% opacity I have to paint about 8 times over to fully cover.
Comment 4 wolthera 2016-01-04 15:21:52 UTC
Created attachment 96448 [details]
File with 50% transparent black layer that doesn't go to 0 even after 10~ layers.

slangkamp, I can't get it to work. I attached a sample file that shows the problem... Color Picking the black square gives 1,1,1.
Comment 5 Dmitry Kazakov 2016-05-26 07:00:56 UTC
Short: 
The solution for the original problem: "just use 51% opacity to avoid rounding, or use another composite op"

Long:

The formula for Normal alpha blending looks like that:

newColor = dstColor + alpha * (srcColor - dstColor)

In our case:
dstColor = 1
srcColor = 0
alpha = 0.5

We do all the calculations in a floating point numbers, so we get:

newColor = 1.0 + 0.5 * (0.0 - 0.5) = 0.5

Then, we convert the floating-point number back to integers using rounding:

newColorInt = round(0.5) = 1

We cannot skip rounding, because if we do the effects will be drastic. The image pixels will flip to different sides in a random way, creating artifacts on the merged image.


So, speaking truly, I don't know if we can (or should) fix this bug. How do the other applications work with this case?
Comment 6 wolthera 2016-05-26 09:01:39 UTC
I can't tell you about GIMP, but MyPaint just does it's normal blending mode in integers:

https://github.com/mypaint/mypaint/blob/master/lib/blending.hpp

(fix_15_t is a uint32_t and fix_15_t_short is a uint16_t)

https://github.com/mypaint/mypaint/blob/master/lib/fix15.hpp

I am not sure what benefit it has to us to use this method anywhere but in the normal/over/general lerp, but for normal/over/lerp it might actually be very useful. The real question I suposse is if this issue is problematic enough in 16bit to have it there as well. (the float spaces don't need it, because well, they're floats)
Comment 7 Halla Rempt 2016-06-23 11:11:31 UTC
We used to do this in integers, afair, and it's still possible to specialize per color model/channel depth. I think we used to have just as many artefacts in the blending brush back then, though, if not more.
Comment 8 Halla Rempt 2016-06-23 11:15:10 UTC
Oh, and just to note: mypaint does all its work internally in 16 bits premultiplied, iirc.
Comment 9 Reinold Rojas 2024-11-12 04:40:04 UTC
This bug looks like its still reproducible on latest master. 

Steps to Reproduce:
1. Change Opacity to 50%
2. Set Color to #000000
3. Draw 10+ strokes over the same spot on canvas
4. Pick color
5. Check color (will be #010101 and Color channels Red/Green/Blue will all be set to 1)
Comment 10 Bug Janitor Service 2024-11-14 10:58:40 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/2268
Comment 11 Dmitry Kazakov 2024-11-18 14:57:37 UTC
Git commit f890fcfd65841d4fe2817b3c12be51e211d35fb3 by Dmitry Kazakov.
Committed on 18/11/2024 at 14:57.
Pushed by dkazakov into branch 'master'.

Switch KisPainter to use floating point opacity instead of 0...255 integer range

Globally, we store opacity as a floating point value. It was quite weird that we
converted the value to quint8, when passing to KisPainter, then back to float when
passing to KoCompositeOp and then, finally, to channels_type.

Now KisPainter operates with floating-point opacity value. It removes all these
extra conversions and errors when rounding.

M  +3    -3    benchmarks/kis_floodfill_benchmark.cpp
M  +1    -1    benchmarks/kis_gradient_benchmark.cpp
M  +1    -1    benchmarks/kis_painter_benchmark.cpp
M  +1    -1    libs/image/kis_fill_painter.cc
M  +4    -4    libs/image/kis_indirect_painting_support.cpp
M  +1    -1    libs/image/kis_indirect_painting_support.h
M  +1    -1    libs/image/kis_layer_projection_plane.cpp
M  +2    -2    libs/image/kis_onion_skin_compositor.cpp
M  +23   -6    libs/image/kis_painter.cc
M  +7    -3    libs/image/kis_painter.h
M  +1    -1    libs/image/kis_projection_leaf.cpp
M  +9    -1    libs/image/krita_utils.cpp
M  +2    -1    libs/image/krita_utils.h
M  +1    -1    libs/image/layerstyles/KisLayerStyleKnockoutBlower.cpp
M  +1    -1    libs/image/layerstyles/kis_layer_style_filter_environment.cpp
M  +1    -3    libs/image/layerstyles/kis_ls_utils.cpp
M  +3    -3    libs/image/lazybrush/kis_colorize_mask.cpp
M  +2    -2    libs/image/tests/kis_layer_style_projection_plane_test.cpp
M  +22   -0    libs/pigment/KoCompositeOp.cpp
M  +13   -0    libs/pigment/KoCompositeOp.h
M  +4    -4    libs/ui/KisView.cpp
M  +3    -4    libs/ui/processing/KisEncloseAndFillProcessingVisitor.cpp
M  +2    -2    libs/ui/processing/KisEncloseAndFillProcessingVisitor.h
M  +4    -4    libs/ui/processing/fill_processing_visitor.cpp
M  +2    -2    libs/ui/processing/fill_processing_visitor.h
M  +1    -1    libs/ui/tool/KisStrokeCompatibilityInfo.cpp
M  +1    -1    libs/ui/tool/KisStrokeCompatibilityInfo.h
M  +7    -9    libs/ui/tool/kis_resources_snapshot.cpp
M  +1    -1    libs/ui/tool/kis_resources_snapshot.h
M  +1    -1    libs/ui/tool/strokes/kis_painter_based_stroke_strategy.cpp
M  +1    -1    plugins/generators/multigridpattern/multigridpatterngenerator.cpp
M  +1    -1    plugins/generators/pattern/patterngenerator.cpp
M  +1    -1    plugins/generators/solid/colorgenerator.cpp
M  +51   -51   plugins/paintops/colorsmudge/KisColorSmudgeStrategyBase.cpp
M  +15   -15   plugins/paintops/colorsmudge/KisColorSmudgeStrategyBase.h
M  +2    -2    plugins/paintops/colorsmudge/KisColorSmudgeStrategyLightness.cpp
M  +13   -14   plugins/paintops/colorsmudge/KisColorSmudgeStrategyMaskLegacy.cpp
M  +4    -4    plugins/paintops/colorsmudge/KisColorSmudgeStrategyMaskLegacy.h
M  +4    -4    plugins/paintops/curvebrush/kis_curve_paintop.cpp
M  +3    -3    plugins/paintops/defaultpaintops/brush/kis_brushop.cpp
M  +1    -1    plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
M  +2    -2    plugins/paintops/deform/kis_deform_paintop.cpp
M  +1    -1    plugins/paintops/gridbrush/kis_grid_paintop.cpp
M  +2    -2    plugins/paintops/hairy/kis_hairy_paintop.cpp
M  +2    -2    plugins/paintops/hatching/kis_hatching_paintop.cpp
M  +6    -6    plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
M  +1    -1    plugins/paintops/libpaintop/KisFlowOpacityOption.h
M  +5    -5    plugins/paintops/libpaintop/KisOpacityOption.cpp
M  +1    -1    plugins/paintops/libpaintop/KisOpacityOption.h
M  +1    -1    plugins/paintops/mypaint/MyPaintSurface.cpp
M  +6    -4    plugins/paintops/sketch/kis_sketch_paintop.cpp
M  +2    -2    plugins/paintops/spray/kis_spray_paintop.cpp
M  +2    -2    plugins/paintops/spray/spray_brush.cpp
M  +2    -2    plugins/paintops/tangentnormal/kis_tangent_normal_paintop.cpp
M  +3    -3    plugins/tools/basictools/kis_tool_fill.cc
M  +1    -1    plugins/tools/basictools/strokes/move_selection_stroke_strategy.cpp
M  +1    -1    plugins/tools/tool_enclose_and_fill/KisToolEncloseAndFill.cpp

https://invent.kde.org/graphics/krita/-/commit/f890fcfd65841d4fe2817b3c12be51e211d35fb3
Comment 12 Dmitry Kazakov 2024-11-18 15:28:41 UTC
Git commit 2028c8f25ecfbfe355d4bbe3c57092ad70da4261 by Dmitry Kazakov.
Committed on 18/11/2024 at 15:07.
Pushed by dkazakov into branch 'krita/5.2'.

Switch KisPainter to use floating point opacity instead of 0...255 integer range

Globally, we store opacity as a floating point value. It was quite weird that we
converted the value to quint8, when passing to KisPainter, then back to float when
passing to KoCompositeOp and then, finally, to channels_type.

Now KisPainter operates with floating-point opacity value. It removes all these
extra conversions and errors when rounding.

# Conflicts:
#	libs/ui/processing/KisEncloseAndFillProcessingVisitor.cpp
#	libs/ui/processing/KisEncloseAndFillProcessingVisitor.h
#	plugins/tools/tool_enclose_and_fill/KisToolEncloseAndFill.cpp

M  +3    -3    benchmarks/kis_floodfill_benchmark.cpp
M  +1    -1    benchmarks/kis_gradient_benchmark.cpp
M  +1    -1    benchmarks/kis_painter_benchmark.cpp
M  +1    -1    libs/image/kis_fill_painter.cc
M  +4    -4    libs/image/kis_indirect_painting_support.cpp
M  +1    -1    libs/image/kis_indirect_painting_support.h
M  +1    -1    libs/image/kis_layer_projection_plane.cpp
M  +2    -2    libs/image/kis_onion_skin_compositor.cpp
M  +23   -6    libs/image/kis_painter.cc
M  +7    -3    libs/image/kis_painter.h
M  +1    -1    libs/image/kis_projection_leaf.cpp
M  +9    -1    libs/image/krita_utils.cpp
M  +2    -1    libs/image/krita_utils.h
M  +1    -1    libs/image/layerstyles/KisLayerStyleKnockoutBlower.cpp
M  +1    -1    libs/image/layerstyles/kis_layer_style_filter_environment.cpp
M  +1    -3    libs/image/layerstyles/kis_ls_utils.cpp
M  +3    -3    libs/image/lazybrush/kis_colorize_mask.cpp
M  +2    -2    libs/image/tests/kis_layer_style_projection_plane_test.cpp
M  +22   -0    libs/pigment/KoCompositeOp.cpp
M  +13   -0    libs/pigment/KoCompositeOp.h
M  +4    -4    libs/ui/KisView.cpp
M  +3    -4    libs/ui/processing/KisEncloseAndFillProcessingVisitor.cpp
M  +2    -2    libs/ui/processing/KisEncloseAndFillProcessingVisitor.h
M  +4    -4    libs/ui/processing/fill_processing_visitor.cpp
M  +2    -2    libs/ui/processing/fill_processing_visitor.h
M  +1    -1    libs/ui/tool/KisStrokeCompatibilityInfo.cpp
M  +1    -1    libs/ui/tool/KisStrokeCompatibilityInfo.h
M  +7    -9    libs/ui/tool/kis_resources_snapshot.cpp
M  +1    -1    libs/ui/tool/kis_resources_snapshot.h
M  +1    -1    libs/ui/tool/strokes/kis_painter_based_stroke_strategy.cpp
M  +1    -1    plugins/generators/multigridpattern/multigridpatterngenerator.cpp
M  +1    -1    plugins/generators/pattern/patterngenerator.cpp
M  +1    -1    plugins/generators/solid/colorgenerator.cpp
M  +51   -51   plugins/paintops/colorsmudge/KisColorSmudgeStrategyBase.cpp
M  +15   -15   plugins/paintops/colorsmudge/KisColorSmudgeStrategyBase.h
M  +2    -2    plugins/paintops/colorsmudge/KisColorSmudgeStrategyLightness.cpp
M  +13   -14   plugins/paintops/colorsmudge/KisColorSmudgeStrategyMaskLegacy.cpp
M  +4    -4    plugins/paintops/colorsmudge/KisColorSmudgeStrategyMaskLegacy.h
M  +4    -4    plugins/paintops/curvebrush/kis_curve_paintop.cpp
M  +3    -3    plugins/paintops/defaultpaintops/brush/kis_brushop.cpp
M  +1    -1    plugins/paintops/defaultpaintops/duplicate/kis_duplicateop.cpp
M  +2    -2    plugins/paintops/deform/kis_deform_paintop.cpp
M  +1    -1    plugins/paintops/gridbrush/kis_grid_paintop.cpp
M  +2    -2    plugins/paintops/hairy/kis_hairy_paintop.cpp
M  +2    -2    plugins/paintops/hatching/kis_hatching_paintop.cpp
M  +6    -6    plugins/paintops/libpaintop/KisFlowOpacityOption.cpp
M  +1    -1    plugins/paintops/libpaintop/KisFlowOpacityOption.h
M  +5    -5    plugins/paintops/libpaintop/KisOpacityOption.cpp
M  +1    -1    plugins/paintops/libpaintop/KisOpacityOption.h
M  +1    -1    plugins/paintops/mypaint/MyPaintSurface.cpp
M  +6    -4    plugins/paintops/sketch/kis_sketch_paintop.cpp
M  +2    -2    plugins/paintops/spray/kis_spray_paintop.cpp
M  +2    -2    plugins/paintops/spray/spray_brush.cpp
M  +2    -2    plugins/paintops/tangentnormal/kis_tangent_normal_paintop.cpp
M  +3    -3    plugins/tools/basictools/kis_tool_fill.cc
M  +1    -1    plugins/tools/basictools/strokes/move_selection_stroke_strategy.cpp
M  +1    -1    plugins/tools/tool_enclose_and_fill/KisToolEncloseAndFill.cpp

https://invent.kde.org/graphics/krita/-/commit/2028c8f25ecfbfe355d4bbe3c57092ad70da4261