Bug 413317

Summary: Height to normal map radius settings don't work properly
Product: [Applications] krita Reporter: Rytelier <rytelierofficial>
Component: FiltersAssignee: Dmitry Kazakov <dimula73>
Status: RESOLVED FIXED    
Severity: normal CC: dimula73, griffinvalley, tysontanx
Priority: NOR    
Version: 4.2.7.1   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In:
Attachments: result at 5.67

Description Rytelier 2019-10-22 14:10:16 UTC
Created attachment 123409 [details]
result at 5.67

SUMMARY
The height to normal map now changes result's intensity after certain values in radius settings are passed. There is no smooth transitions between output's strength. After 5.66 value, it turns all blue.

STEPS TO REPRODUCE
1. Open some texture
2. Use height to normal map filter
3. Adjust the radius

OBSERVED RESULT
Radius slider providing normal strength change only after certain values. Map turn fully blue after 5.66

EXPECTED RESULT
Slider provides smooth results according to the radius.

SOFTWARE/OS VERSIONS
Windows: 10
Comment 1 wolthera 2019-10-22 14:15:40 UTC
> now changes

Was it different in previous versions?
Comment 2 Rytelier 2019-10-22 14:19:34 UTC
(In reply to wolthera from comment #1)
> > now changes
> 
> Was it different in previous versions?

In 4.2.5 (I think) the filter did smoother results with the radius slider. After the update, all normal map filter masks I had in files broke.
Comment 3 wolthera 2020-07-28 11:25:25 UTC
*** Bug 423094 has been marked as a duplicate of this bug. ***
Comment 4 wolthera 2020-07-28 11:30:34 UTC
So, Ahab noticed the following that it triggers at:

 5.67 px at 1024 x 1024 canvas size
11.34 px at 2048 x 2048 canvas size
22.67 px at 4096 x 4096 canvas size

This seems to suggest the problem is specifically at the edge between filter tiles...
Comment 5 wolthera 2020-08-23 12:14:56 UTC
So... bisecting says that this is the bad commit:

5ccac066cd8d98e655b7d14349dc011205ac096f is the first bad commit
commit 5ccac066cd8d98e655b7d14349dc011205ac096f
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Mon Sep 16 21:14:41 2019 +0300

    Fix wrong borders on EdgeDetection and HeightToNormalMap filters
    
    The color data should be premultiplied by the alpha channel to
    get correct result on layers with transparency.
    
    BUG:411922

 libs/image/kis_edge_detection_kernel.cpp                 | 16 ++++++++++++++++
 libs/pigment/KoColorSpace.h                              |  5 +++++
 libs/pigment/KoColorSpaceAbstract.h                      |  5 +++++
 .../kis_convert_height_to_normal_map_filter.cpp          |  3 ---
 .../filters/edgedetection/kis_edge_detection_filter.cpp  |  3 ---
 5 files changed, 26 insertions(+), 6 deletions(-)

(https://invent.kde.org/graphics/krita/-/commit/5ccac066cd8d98e655b7d14349dc011205ac096f)

I guess premultiplying is what leads to this bug???
Comment 6 Dmitry Kazakov 2020-08-24 22:39:49 UTC
Git commit bc8f87a0ea5f84f4b42f0ae77d9162009eab79ed by Dmitry Kazakov.
Committed on 24/08/2020 at 22:27.
Pushed by dkazakov into branch 'krita/4.3'.

Fix a bug in KisConvolutionWorkerFFTW

Kernel's m_absoluteOffset[channel] should be added after we
unmultiply alpha. Doing otherwise breaks edge detection kernels.

M  +6    -1    libs/image/kis_convolution_painter.cc
M  +5    -3    libs/image/kis_convolution_painter.h
M  +1    -1    libs/image/kis_convolution_worker_fft.h
M  +1    -1    libs/image/kis_convolution_worker_spatial.h
M  +12   -1    libs/image/kis_edge_detection_kernel.cpp
M  +3    -1    libs/image/kis_edge_detection_kernel.h
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_0.5_0.5_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_0.5_0.5_spatial.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1.5_1.5_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1.5_1.5_spatial.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1_1_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1_1_spatial.png.png
M  +93   -2    libs/image/tests/kis_convolution_painter_test.cpp
M  +5    -0    libs/image/tests/kis_convolution_painter_test.h

https://invent.kde.org/graphics/krita/commit/bc8f87a0ea5f84f4b42f0ae77d9162009eab79ed
Comment 7 Dmitry Kazakov 2020-08-24 22:40:04 UTC
Git commit db49b3e1df72fae83467162252a55be8e29c6557 by Dmitry Kazakov.
Committed on 24/08/2020 at 22:39.
Pushed by dkazakov into branch 'master'.

Fix a bug in KisConvolutionWorkerFFTW

Kernel's m_absoluteOffset[channel] should be added after we
unmultiply alpha. Doing otherwise breaks edge detection kernels.

M  +6    -1    libs/image/kis_convolution_painter.cc
M  +5    -3    libs/image/kis_convolution_painter.h
M  +1    -1    libs/image/kis_convolution_worker_fft.h
M  +1    -1    libs/image/kis_convolution_worker_spatial.h
M  +12   -1    libs/image/kis_edge_detection_kernel.cpp
M  +3    -1    libs/image/kis_edge_detection_kernel.h
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_0.5_0.5_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_0.5_0.5_spatial.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1.5_1.5_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1.5_1.5_spatial.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1_1_fftw.png.png
A  +-    --    libs/image/tests/data/convolution_painter_test/normalmap_reduced_test_1_1_spatial.png.png
M  +93   -2    libs/image/tests/kis_convolution_painter_test.cpp
M  +5    -0    libs/image/tests/kis_convolution_painter_test.h

https://invent.kde.org/graphics/krita/commit/db49b3e1df72fae83467162252a55be8e29c6557