Open kleinesfilmroellchen opened 2 years ago
The filter architecture gave me a headache since I updated it for the Gallery. I love seeing the suggestions on this! I'm not too sure whether moving all the filters over to LibGfx is possible, let's see where this goes when we come to it.
Great having people care about PP :^)
While working on #14840, I noticed several problems with the current PixelPaint filter pipeline:
apply
API receives C++ references to bitmaps, making it impossible to use cheapmove
s for temporary bitmaps.apply
API's source and target bitmap are almost always aliased (which C++ allows, which is a language flaw we need to work around here), meaning that a copy (which cannot be moved as discussed above) is always necessary.apply
calls happen on the GUI thread, causing hangs for compute-heavy filters.This causes performance problems and necessitates unclean workarounds. I therefore suggest the following rearchitecting, which can for the most part happen in small modular steps:
PixelPaint::Filter::apply()
non-virtual to enforce correct and uniform handling of source and target bitmaps as well as the edit stack.apply(NonnullRefPtr<Bitmap>, NonnullRefPtr<Bitmap const>)
and never call it with aliased smart pointers; i.e. users can overwrite the target however they like and assume it starts out with the same size and format as the source.FilterParameters
).PixelPaint::Filters::LibGfxFilter
templated mixin class that delegates to a LibGfx filter and handles 90% of simple filters.CC @TobyAsE I want to hear what's your opinion on this.