| Summary: | Artifacts in Emboss with Variable Depth filter. | ||
|---|---|---|---|
| Product: | [Applications] krita | Reporter: | Spencer Brown <sbrown655> |
| Component: | Filters | Assignee: | Krita Bugs <krita-bugs-null> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | halla |
| Priority: | NOR | ||
| Version: | git master (please specify the git hash!) | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Other | ||
| Latest Commit: | Version Fixed In: | ||
|
Description
Spencer Brown
2014-03-03 06:39:33 UTC
Yes. I see what you mean. I'm going to take a look at your code. Hm, well -- using KisSequentialIterator is correct -- I cannot reproduce artefacts with your filter (I woudnt have expected them, either). Your other option would be something like is done in kis_unsharp_filter, where a hline iterator is created and then we loop over the rows:
KisHLineIteratorSP dstIt = device->createHLineIteratorNG(rect.x(), rect.y(), rect.width());
quint8 *colors[2];
colors[0] = new quint8[pixelSize];
colors[1] = new quint8[pixelSize];
for (int j = 0; j < rect.height(); j++) {
do {
...
} while (dstIt->nextPixel());
dstIt->nextRow();
}
But that is actually less efficient -- though it might be necessary if you use more than one paint device at the same time.
The big problem with the emboss filter is that it needs pixel information outside the rect that it gets; krita filters can be applied tile-wise, without the filter know anything about that. So if your filter needs access to pixels outside the rect it is allowed to change, it has to ask for that. Apart from that, the ancient code Michael Thaler copied ten years ago from Digikam has more problems -- I see some overflows as well. Let's see what can be salvaged. Thanks, that helps a lot with what I'm working on. My filter doesn't make much sense without write access to all the pixels in the image, so I think I'll have to make sure it requests sequential access properly. After I've got it working and the patch submitted I may go back and see if I can improve performance by computing both passes in the Euclidean distance transform at once, then combining the results for each pass. If you add setSupportsThreading(false); in the constructor, the filter will always get the whole image in one go Git commit d482cd2ec24db1f2dfe7ee23b5b35ab3117d3f1f by Boudewijn Rempt. Committed on 03/03/2014 at 20:17. Pushed by rempt into branch 'master'. The emboss filter needs access to the whole image Note: there still are some weird things, like color pixels showing up... M +1 -0 krita/plugins/filters/embossfilter/kis_emboss_filter.cpp http://commits.kde.org/calligra/d482cd2ec24db1f2dfe7ee23b5b35ab3117d3f1f Git commit 6ccdc18bf2e6856e656b8ed59b63577961d005c1 by Boudewijn Rempt. Committed on 03/03/2014 at 20:17. Pushed by rempt into branch 'calligra/2.8'. The emboss filter needs access to the whole image Note: there still are some weird things, like color pixels showing up... M +1 -0 krita/plugins/filters/embossfilter/kis_emboss_filter.cpp http://commits.kde.org/calligra/6ccdc18bf2e6856e656b8ed59b63577961d005c1 |