Re: more visiting and less buffering in drawinglayer/

[Luboš Luňák <l.lunak@xxxxxxxxxxxxx>]

On Friday 03 of December 2021, Armin Le Grand wrote:
> In this case it's - as you say - broken and thus an error. So please
> talk about it, report it

 How likely would that achieve anything though? It's not like reporting a 
problem somehow magically fixes it. Even high-priority bugreports with a 
simple solution like tdf#138068 can apparently linger for quite while, so 
what's the chance a non-critical problem with a complex fix would do anything 
else than sit in bugzilla and do nothing?

> and fix it when it's an error - or motivate others to fix it.

 Given the above, that's what I normally try to do, but that's not always 
possible. In the case of the canvas caching, after several attempts at it I 
managed to fix like 3(?) places that were breaking it, and after I ran into 
yet another reason why it was irrelevant I just gave up. As for motivating 
others, I don't see how that'd happen given 'git log canvas cppcanvas' - 
ironically the largest recent contributor of commits specific to that code is 
probably me, and the only thing I feel like motivating people to do is to 
rewrite code using it away from it.

> I think it's not the most effective way if everyone 
> implements it's own buffers. This is possible (as we see), but I doubt
> that it's effective - except that you do not need to cope too much with
> the existing stuff ;-) It will even eventually conflict potentially in
> the long run and wastes lots of energy :-( The ype of energy that is the
> most valuable for the project - developer ressources.
>
> Please note that there is nowadays type-template-based buffering of
> system-dependent data - of *any* kind. That works for win/cairo/X11
> specific bitmap representations. It also is used for buffering
> system-dependent PolyPolygon path representations (cairo & win) which
> are also expensive to re-create at each repaint. I see no hindrance to
> use the same system for Skia. This is not complicated, just needs a
> little research/looking (or asking & help) how it is used. It even can
> have it's own 'decider' if it should be buffered longer or can be
> dismissed.
>
> If there are cases for (software-)scaled/changed bitmaps for paint which
> are not yet handled - and there are always such, of course - please
> report them. This system can be used to do this, dependent of
> attributes/necessities of your choice.

 Actually that's how I saw it in theory too, and that's where I started, but 
then practice went rather differently. As I recall it, it could be summed 
roughly as:
- it's not documented (can't help notice the irony of you talking about not 
wasting developer time when you didn't find 5 minutes to do something about 
it)
- the usage didn't seem clear from looking at the 3 classes
- the usage didn't seem clear from looking at how VCL cairo/headless backend 
uses it (IIRC the main thought I had was that it seemed rather complicated 
for something as simple as caching)
- after deciphering some of it the assumptions seemed wrong (only one item of 
the same type for an object, so no caching of scaling to different sizes; 
binding the data to the one object, so problematic when copying bitmaps or 
involving more than one of them)
- it seemed that trying to save time/code by reusing the code would result in 
writing more code than would be saved
- when I tried the cairo backend with whichever bugreport I needed the caching 
for, it performed poorly
- so when faced with the decision whether to write a simple cache myself or 
try to fit a complex undocumented design that probably would work poorly, I 
indeed decided not to waste developer time

 Even going over this again in hindsight I still think that was the correct 
and effective decision - AFAICT the Skia backend generally outperforms the 
Cairo one, and it's not just because of Skia itself. As far as I'm concerned, 
this is the common OOo problem of something looking nice in theory but 
struggling with the reality check.

-- 
 Luboš Luňák
 l.lunak@xxxxxxxxxxxxx