392191 2018-03-22 19:57:04 +0000 Height Map to Normal Map creates a line at the top/bottom edge. 2018-04-03 11:47:06 +0000 1 1 2 Applications krita Filters 4.0 unspecified All RESOLVED FIXED NOR normal --- 1 sidartalei krita-bugs-null dennis.ranke halla https://commits.kde.org/krita/a0e2a23f0e7ab1054b9fd49cad29c3c58590cacc 0 oldest_to_newest 1739840 0 111558 sidartalei 2018-03-22 19:57:04 +0000 Created attachment 111558 The canvas in wrap mode, the lines become apparent. The Height Map to Normal Map filter creates height information on the top and bottom of the image. 1739841 1 griffinvalley 2018-03-22 19:59:04 +0000 Yes, this is as far as I understand a problem with the convolution painter itself. I pass the wraparound variable correctly, as far as I know. 1739975 2 dennis.ranke 2018-03-23 11:15:59 +0000 I did a read through the code and I /think/ I identified two issues that relate to this bug: 1. In KisEdgeDetectionKernel::convertToNormalMap there is this code to apply the horizontal and vertical convolution kernels: horizPainterLR.applyMatrix(kernelHorizLeftRight, device, srcTopLeft - QPoint(0, ceil(horizontalCenter)), srcTopLeft - QPoint(0, ceil(horizontalCenter)), rect.size() + QSize(0, 2 * ceil(horizontalCenter)), BORDER_REPEAT); and verticalPainterTB.applyMatrix(kernelVerticalTopBottom, device, srcTopLeft - QPoint(0, ceil(verticalCenter)), srcTopLeft - QPoint(0, ceil(verticalCenter)), rect.size() + QSize(0, 2 * ceil(verticalCenter)), BORDER_REPEAT); If you look at the QPoints and the QSize, you see that in both cases the x is set to zero and the y to some value based on the horizontal/vertical center. I guess that in one of the cases, probably the vertical one, the two vector components should be flipped around. 2. The definition of BORDER_REPEAT which you pass into applyMatrix is defined as: BORDER_REPEAT = 1 // Use the border for the missing pixels Which is actually a clamp, not a wrap-around. So a wrapping normal map will have some artifacts on the borders. Ideally, the filter would gain an option to select a newly coded BORDER_WRAP mode, although that's really more of a feature request rather than a bug. 1739976 3 dennis.ranke 2018-03-23 11:21:37 +0000 Ok, I guessed wrong, it's actually the horizontal applyMatrix that is wrong: horizPainterLR.applyMatrix(kernelHorizLeftRight, device, srcTopLeft - QPoint(ceil(horizontalCenter), 0), srcTopLeft - QPoint(ceil(horizontalCenter), 0), rect.size() + QSize(2 * ceil(horizontalCenter), 0), BORDER_REPEAT); gets rid of the line at the top and bottom edges for me. 1741326 4 halla 2018-03-28 14:08:56 +0000 Git commit bc30d50e80024d7b2352fa0b3112ecff5e9c36aa by Boudewijn Rempt. Committed on 28/03/2018 at 14:08. Pushed by rempt into branch 'master'. Fix height map to normal map in wraparound mode patch by Dennis Ranke. Thanks! M +3 -3 libs/image/kis_edge_detection_kernel.cpp https://commits.kde.org/krita/bc30d50e80024d7b2352fa0b3112ecff5e9c36aa 1742850 5 halla 2018-04-03 11:47:06 +0000 Git commit a0e2a23f0e7ab1054b9fd49cad29c3c58590cacc by Boudewijn Rempt. Committed on 03/04/2018 at 11:18. Pushed by rempt into branch 'krita/4.0'. Fix height map to normal map in wraparound mode patch by Dennis Ranke. Thanks! (cherry picked from commit bc30d50e80024d7b2352fa0b3112ecff5e9c36aa) M +3 -3 libs/image/kis_edge_detection_kernel.cpp https://commits.kde.org/krita/a0e2a23f0e7ab1054b9fd49cad29c3c58590cacc 111558 2018-03-22 19:57:04 +0000 2018-03-22 19:57:04 +0000 The canvas in wrap mode, the lines become apparent. Krita.png image/png 1537 sidartalei iVBORw0KGgoAAAANSUhEUgAAAL8AAAHiCAIAAAA274rMAAAAAXNSR0IArs4c6QAAAARnQU1BAACx jwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAASdEVYdFNvZnR3YXJlAEdyZWVuc2hvdF5VCAUA AAV4SURBVHhe7dpLjptAFEBR7ysbyipC1pZd9LT3kGkntLDwJ3aA61nXkY4YQM24ourJPk3Tx35/ fv64u8PIjtUzExCrw/XMBMSi1DMTEDP10MV6ZgJCPXS9npmABndTz9Ea1DO4cz1rBwJiv896rgtQ D/ud7l6/etjvpXPPQkDDUg/dpZ4cgXqGdfPtadQzrHM9rxSgnmGdZ675urh+ttiMY3MBX9X9zrU2 dH39P/UM6+m5Z21iMw71DGu7nk3qGZZ66C4z1511xaZDi/lKPut58fWrZ1iXb8966yj1DOty7skR qGdYN6fm0IF0RnZTz+xoDeoZ2ecvFa8UoJ6RPZi59gchncG9NHOpZ3A3M9dSw84mpMP9qXk/9RDr kQ4z9dCVeqTD4nA90mF1rB7pcO1APdLhzt56pMO/dtUjHR7aqGfuRjo887Qe3bDpQT26YadLPUs0 umG/87/DREOwa+aCh9RDpx469dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EN3mr69 Q3Oafr1DY+eiUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deujUQ6ceOvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx46 9dCph049dOqhUw+deuhO0/ff0Jymtzdo7Fx06qFTD5166NRDpx469dCph049dOqhUw+deujUQ6ce OvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx469dCph049dOqhUw+deujUQ6ce OvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD5166NRDpx469dCph049dOqhUw+deujUQ6ce OvXQqYdOPXTqoVMPnXro1EOnHjr10KmHTj106qFTD9X08Ret8KzUd55WSQAAAABJRU5ErkJggg==