Implement reoriented normal map combine blending mode in Krita
ClosedPublic
Actions

Authored by sbrown on Jul 30 2015, 3:45 PM.

Details

Reviewers

Summary

Blending In Detail details an algorithm for combining two normal maps in a mathematically correct way. I've implemented it as a pigmentcms blending mode and it works pretty well as a blending mode for both layers and Wolthera's tangent normal brush. The implementation is pretty rough around the edges still--everything's in one method, I haven't done any profiling or SIMDization yet, the data are reshuffled rather than working in the colorspace's native format, I'm not 100% sure that blending works correctly in all cases etc. etc. but I need to go eat lunch so those enhancements will have to wait for a bit.

The blending mode currently enforces use in the RGB spaces, which I believe to be the correct behavior.

Test Plan

make sure that transparency and blending work sanely in all cases
see what disabling channels does to it (honestly I'm not sure what it is supposed to do, it's not as clear cut as other blending modes since the channels represent directions that depend on each other for orientation)
check how it works in non-8-bit color spaces
break out logical blocks into methods
run a profiler on an optimized build and see where the hot-spots are in a real-world test case
optimize and SIMDize accordingly if there are hot-spots in the blending mode

Diff Detail

Repository

R8 Calligra

Lint

Lint Skipped

Unit

Unit Tests Skipped

sbrown updated this revision to Diff 468.Jul 30 2015, 3:45 PM

sbrown retitled this revision from to Implement reoriented normal map combine blending mode in Krita.

sbrown updated this object.

sbrown edited the test plan for this revision. (Show Details)

sbrown added a reviewer: woltherav.

sbrown set the repository for this revision to R8 Calligra.

sbrown added a project: Krita.

sbrown added subscribers: rempt, jringstad, lukast.

I need to wake up a bit more, but here's a starter.

libs/pigment/compositeops/KoCompositeOpReorientedNormalMapCombine.h
39 ↗	(On Diff #468)	It seems to me that this is a problem getting it to work on higher bitdepths. To wit, this is what dissolve uses: static const qint32 alpha_pos = Traits::alpha_pos; and generic also uses qint32, because that's the maximum Krita supports right now. (dunno, why behind uses qint8...)

Okay, I did some experimenting:

normalsI16.kra979 KBDownload

Here, I switched the Red and Blue channels, which seems to have a good effect. But then you are still dealing with a inverted green for 16bit, which I inverted with the invert filter masks, but hey, guess what, invert filter is broken for 16bit. Regardless, it seems to get close in principle.

I am a bit surprised that RGBA works for you with the 8bit space, because for the tangent normal brush I had to use BGRA as well... Maybe because I was using the a defined color space?
edit: please reference kocompositeopgeneric.h to get the components in the right place :) (There's stuff in there for alpha blending and the like as well)

libs/pigment/compositeops/KoCompositeOpReorientedNormalMapCombine.h
39 ↗	(On Diff #468)	okay, tried this, had little effect. But still a good idea for precision's sake...
139 ↗	(On Diff #468)	I am pretty sure we use BGRA instead of RGBA for 16bit, may also be extremely related. (Edit: it's related, yes)

Simplify RNM blending mode to use KoCompositeOpGeneric

This fixes all problems with alpha blending, substantially shortens the code, and makes the blending mode work correctly on all bit depths! It probably runs faster as well, though I can't say that for sure as I haven't profiled. Thanks, Wolthera, for putting me on the right track.

Forgot to delete an include directive. How embarrassing! With the new diff everything should build correctly.

In D210#3912, @sbrown wrote:

Forgot to delete an include directive. How embarrassing! With the new diff everything should build correctly.

You made a mistake in making the diff too. Your diff doesn't apply either. You seem to make a diff of your already existing stuff...

recombinedblendingpatch.diff3 KBDownload

I frankensteined together a clean diff. How about you clean up your working directory so it's easier to make diffs and then apply this one and work from there.

I tested it, and everything looks fine, so it can actually be commited to (preferably) calligra/2.9. I would actually recommend doing that(you still have commiter access from gsoc last year, yes?) and start polishing afterwards.

Thing is, we will have new builds coming week, so your blending mode will be able to be tested then. And I would also like to keep my GSoC branch to only have unique commits for myself so that it's easier to make a diff for the end of the period.

edit: btw, when commiting write ' Differential: https://phabricator.kde.org/D210 ' in the task, that should close this one.

This revision is now accepted and ready to land.Aug 1 2015, 1:23 PM

So I should make diffs that apply only to the original revision that they're based off of. Got it.

I will see if I still have committer access and get this pushed if I do.

One last question before I push: what about the tangent normal map blending mode that's in master? Should I remove that from master and replace it with my patch or just leave it alone for now?

In D210#3920, @sbrown wrote:

One last question before I push: what about the tangent normal map blending mode that's in master? Should I remove that from master and replace it with my patch or just leave it alone for now?

let me deal with it. Push the patch to calligra/2.9, not master.

OK.

aacid closed this revision.Feb 17 2017, 1:53 PM

Revision Contents
Changeset List

			Path	Packages
M			libs/pigment/compositeops/KoCompositeOps.h (1 line)

Diff 509

View Options

libs/pigment/compositeops/KoCompositeOps.h

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	468		Jul 30 2015, 3:31 PM	★	★
Diff 2	508		Jul 31 2015, 10:46 PM	★	★
Diff 3	509		Aug 1 2015, 4:59 AM	★	★