Painting performance improvements - Tiled buffer

[MrStevns]

#1760

This PR introduces the use of a tiled buffer, which effectively makes painting faster and more robust. Both the Vector and Bitmap backends can utilize it with minimal changes and the output.

Motivation

One of my pet peeves with the current painting backend is that we re-allocate too much and modify the current BitmapImage implicitly through bound updates way to often, potentially on every draw operation. The result of this is that the application freezes for anything between a few hundred milliseconds to multiple seconds if the update surface is large enough, making it near impossible to draw on larger surfaces.

Q: Can't we just get rid of bound updates then?
A: Well no because we do need to update the image at some point, when you draw outside its allocated region, otherwise the stroke you've made will be cut off. Ideally it should be possible to delay this operation till we're done drawing.

The freezing can to some extend be worked around by always allocating an image which is big enough, for example the size of the camera, however once you paint outside that area, the problem appears again. While that could have been a decent compromise too, I also feel that if we want to embrace our infinite canvas, we can and should do better.

I've experimented with a tiled painter for a long time, in many different variations, mostly through the mypaint branch but also through other attempts at improving the performance of the canvas. Most of my other attempts has required major changes to the overall painting logic and use of tools. The following proposal however require minimal changes to the painter and tools.

Although I should mention that this still won't allow for a truly infinite canvas, since we're still relying on a single qimage in the end for the final composition, which turns problematic once you start working on a canvas larger than 10000x10000 afaik.

Proposal

Introducing 🥁 🥁 🥁 TiledBuffer
I've taken inspiration from MyPaint where a tile is allocated of a fixed size, in this and our case 64x64 pixels. We can change this later if we find that there's a better size but for now it works fine, so I've seen no reason to change it.
The class is meant to replace BitmapImage as the temporary Bitmap buffer and be more or less interchangeable with what we had.

The way it works:

When you lay down a stroke, the tiled buffer will either look for a tile which may exist already or create a new one, the tile then becomes a temporary drawing buffer where the stroke will be drawn into.
This can be done through any of these methods:

    void drawBrush(const QPointF& point, int brushWidth, QPen pen, QBrush brush, QPainter::CompositionMode cm, bool antialiasing);
    void drawPath(QPainterPath path, QPen pen, QBrush brush,
                  QPainter::CompositionMode cm, bool antialiasing);
    void drawImage(const QImage& image, const QRect& imageBounds, QPainter::CompositionMode cm, bool antialiasing);

Since there may be several tiles you need to paint to, we go through all the ones that may fit your input brush, path or image. When that's done we notify the ScribbleArea that the TiledBuffer has been updated, which in turn will call update, mark the region as dirty and then repaint through ScribbleArea::paintEvent. Here the painter will try to update only based on the bufferRect that we've specified, this should at minimum be the size of the tile but can also be larger if we've touched multiple tiles.

When we're loading an image or drawing somewhere where there's pixels already but no tile data, the loadTile method will make sure to paint the current BitmapImage onto the TiledBuffer, painting only what we're modifying, the rest is kept through the pixmap cache.

Once you stop drawing on the canvas, the buffered content is applied to the respective keyframe and the tiledbuffer is cleared.

Expectations

While I haven't made any benchmarks yet, I can say that from my own experience, i've been able to draw on a surface as large as 4k with no apparent lag. Compared to before where I could draw around a region of ~600-800 pixels before it would start freezing.

Let me hear your thoughts when you've tried it out 😄

Updated July 23 - 12:22.
Linux: https://get.pencil2d.org/@pencil2d/5635833107/pencil2d-linux-625-2023-07-23
Mac: https://get.pencil2d.org/@pencil2d/5635833107/pencil2d-mac-625-2023-07-23
Win32: https://get.pencil2d.org/@pencil2d/5635833107/pencil2d-win32-625-2023-07-23
Win64: https://get.pencil2d.org/@pencil2d/5635833107/pencil2d-win64-625-2023-07-23

[J5lx]

Not sure how to reproduce this one. Do you draw a stroke while moving the cursor outside the window and come back in?

I just realised the lagging in my video made that difficult to see, sorry about that. But yes, that is how I’m triggering that behaviour. I did some additional testing and realised this does not seem to happen on Windows or Qt 6, so I can only assume this is a Linux-specific bug or quirk in Qt 5 that was fixed in Qt 6. The workaround you suggested sounds promising.

Unfortunately I also uncovered some additional issues while investigating this… I don’t really have the time or peace of mind to properly document them right now and to reply to your other comments, so I’ll do that in a few days.

[MrStevns]

Just a small update based on my findings from my post on Discord.

Sigh... I compiled the canvas improvement PR against Qt 6, which fortunately fixes the weird pixel inconsistency I have been haunted by and Jacob also mentioned... but of course Qt wouldn't just work with that, now I just have another issue... which is that drawing at various zoom levels will now reveal the seams of the tiles... 😩

This seems to be a Qt rendering bug and I haven't been able to find a workaround, so it's a blocking issue in terms of this feature when compiling against Qt 6.
I've been able to reproduce the problem in a small example, so a bug report has been created.
https://bugreports.qt.io/browse/QTBUG-116297

[J5lx]

Ok so, the additional issues I found are basically more ways to trigger the same kind of drawing bug as the moving-the-cursor-outside-scribblearea issue from my original review. Specifically, when:

• Pressing Alt while drawing to activate the eyedropper temporary tool
• Pressing and releasing Ctrl+Shift while drawing to (de)activate the eraser temporary tool
  • Note that this won’t actually (de)activate the tool, it only results in this rendering issue
• Pressing and releasing any tool shortcut while drawing
  • This also doesn’t switch the tool but nevertheless results in the rendering issue

The last issue I noticed was the Qt 6 seams one which you already discovered yourself, let’s hope that bug report you made is addressed soon.

[MrStevns]

Ok so, the additional issues I found are basically more ways to trigger the same kind of drawing bug as the moving-the-cursor-outside-scribblearea issue from my original review. Specifically, when:

* Pressing Alt while drawing to activate the eyedropper temporary tool

* Pressing and releasing Ctrl+Shift while drawing to (de)activate the eraser temporary tool
  
  * Note that this won’t actually (de)activate the tool, it only results in this rendering issue

* Pressing and releasing any tool shortcut while drawing
  
  * This also doesn’t switch the tool but nevertheless results in the rendering issue

The last issue I noticed was the Qt 6 seams one which you already discovered yourself, let’s hope that bug report you made is addressed soon.

I've experimented a bit here and I think that not clearing the cache till we're done drawing is the best action we can take.

Another thing i should mention regarding this PR, the unofficial drawing while playback is active is also not working anymore but then again it was never regarded as a real feature, so maybe that's fine. Allowing such a feature again will be difficult as it is, because we're first applying the paint to the BitmapImage on endStroke.

We should probably stop playback when a stroke has been started, to avoid other painting problems.. 🤔

[J5lx]

I’ve tested your changes and they do fix the issue with the Ctrl-Shift eraser shortcut and the regular tool shortcuts, but the issue persists with the Alt eyedropper shortcut or when the cursor moves outside ScribbleArea on Linux Qt 5.

the unofficial drawing while playback is active is also not working anymore but then again it was never regarded as a real feature, so maybe that's fine

I agree. I don’t think we should worry too much about that.

[MrStevns]

I’ve tested your changes and they do fix the issue with the Ctrl-Shift eraser shortcut and the regular tool shortcuts, but the issue persists with the Alt eyedropper shortcut or when the cursor moves outside ScribbleArea on Linux Qt 5.

the unofficial drawing while playback is active is also not working anymore but then again it was never regarded as a real feature, so maybe that's fine

I agree. I don’t think we should worry too much about that.

I've tried to reproduce the eyedropper clearing bug but that too seems to work fine on mac. I'll look around and see if I can figure out why that would happen.

Maybe it's a mac thing but I can't even trigger eyedropper while drawing.

[J5lx]

I did some debugging and managed to fix the issue with Alt and the cursor leaving ScribbleArea. It was caused by CanvasPainter::paintCurrentBitmapFrame assuming that the tiled buffer covered the entire blit rect, which isn’t necessarily the case. I submitted a proposed fix at MrStevns#21.

[J5lx]

Looking pretty good overall, just a handful of minor notes now. Looking forward to merging this soon :)

[J5lx]

Looking good now, I have no further comments. Feel free to merge yourself if you have no more adjustments to make.

[MrStevns]

Nice, thanks! I'll merge it then 🙌