2nd try of fixing color fluctuations in colorsmudge-dulling brushes
ClosedPublic

Authored by dkazakov on Jan 5 2018, 7:09 PM.

Details

Summary

Now the colorsmudge brush does all the blending in the high bit-depth colorspace. The patch introduces a special wrapper (KisPrecisePaintDeviceWrapper) that ensures a proper color space is used when needed. See the apidox in KisPrecisePaintDeviceWrapper.h.

NOTE: since version 2 the patch fixes both dulling and smearing modes!

BUG:348267

Test Plan

Needs to be tested:

  1. Colorsmudge brushes in Dulling or Smearing modes with Color Rate option enabled
  2. Smudge Radius enabled or disabled: both should work correctly
  3. Opacity option should work correctly
  4. Overlay mode?

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dkazakov requested review of this revision.Jan 5 2018, 7:09 PM
dkazakov created this revision.
woltherav added a subscriber: woltherav.EditedJan 5 2018, 9:54 PM

I am sorry Dmitry, but I am still able to reproduce the error fully with the same black, the same orange, and the default block_wet_tilt with brush-tip density set to 100%.

It is just really a nasty interplay between the opacity, the smudge brush sampling the dabs it just laid down and 8bit being too limited to handle this.

EDIT: Please don't spend too long on this bug, it IS really nasty, and we've been suspecting for years other application just have a very different design to their smudge brushes to get around this issue(like, for example, 16bit compositing).

dkazakov added a comment.EditedJan 6 2018, 10:20 AM

I am sorry Dmitry, but I am still able to reproduce the error fully with the same black, the same orange, and the default block_wet_tilt with brush-tip density set to 100%.

Hi, Wolthera!

Could you please attach a screenshot/kra-file and a preset file? :)

EDIT: I can only see reddishness, without severe fluctuations like were present in the original preset. And the funny thing is: if you set spacing to 5%, it becomes a bit better :)

No, I cannot because I reverted the patch and it takes forever to build on this device. The brush I specified is a default brush. Just modify it, and use the same orange and black.

Scott will be merging in the new presets, so I am attaching the faulty brush here:

dkazakov updated this revision to Diff 24995.Jan 9 2018, 10:18 AM

The patch is completely rewtitten. Now colorsmudge op uses higher
bit depth for all the rendering.

dkazakov retitled this revision from Fix color tone fluctuations in colorsmudge-dulling brushes to 2nd try of fixing color fluctuations in colorsmudge-dulling brushes.Jan 9 2018, 10:19 AM
dkazakov edited the summary of this revision. (Show Details)
dkazakov edited the test plan for this revision. (Show Details)
dkazakov edited the summary of this revision. (Show Details)
dkazakov updated this revision to Diff 25016.Jan 9 2018, 1:27 PM
  • Optimize KisPrecisePaintDeviceWrapper by using strides
  • Correctly initialize the precise device
  • Don't try to read area that is empty :)
NOTE: now the speed difference is almost indistinguishable
woltherav accepted this revision.Jan 9 2018, 4:13 PM

This works for me :)

This revision is now accepted and ready to land.Jan 9 2018, 4:13 PM
woltherav added a comment.EditedJan 9 2018, 6:50 PM

Okeys, I found a bug: composite op application got refactored out with the changes, which means that the erase mode doesn't work, amongst others.

Edit: If this makes it easier, precision errors in composite ops besides normal/source-over are not as necessary to be fixed as it isn't expected that the result is precisely in the middle of the two mixed colors. I'll leave it up to you how you want to solve this.

plugins/paintops/colorsmudge/kis_colorsmudgeop.cpp
321

One teensy problem here: The composite opp doesn't apply. So that means I tried to use "erase" and suddenly it didn't work.

This revision was automatically updated to reflect the committed changes.