Skip to content

Improve performance of fill tools

Deif Lou requested to merge deiflou/krita:deiflou/fill_tool into master

This set of patches originated as a reaction to this thread in KA: https://krita-artists.org/t/fill-tool-lag-again-and-again/89562

In short, the fill tools computation time seemed to be proportional to the image size. This should not be the case, it should be roughly proportional to the number of pixels filled.

So, this is what I did (explained by commit):

  • Fix issue with invert selection on KisEncloseAndFillPainter. This is actually unrelated to the original issue, but also produced a delay proportional to image size. There was a performance penalty when using KisPixelSelection invert operations in the enclose and fill tool.
    The tool may generate a small temporary mask that has to be inverted and then intersected with the original enclosing mask that the user makes. The issue is that the inverted mask now has the same bounds as the image, which may be large, and the intersection then takes a lot of time. To solve that I introduced a custom invert mask operation that only inverts the pixels in a specific rect.
  • Make the scanline fill compute the bounding rect of the filled region. This changes KisScanlineFill so that a rect with the bounds of the filled region is maintained. It is needed for the optimization in the next commit.
  • Improve performance of the fill tools by updating only the rect that was changed. This was the main issue to solve. Basically, the fill tool, enclose and fill tool, and "drag & drop color on the canvas to fill" were updating the whole canvas after filling. This made the operation time be somewhat proportional to the image size. On large images (around 7000px here) there was a lag of maybe around 1 second.
    I changed the fill and enclose and fill processing visitors to accept an "out update rect" that they can change to reflect the size of the region filled.
    For the fill tool and the drag and drop feature the change was just using that rect on the update command. Another issue here was the the fill tool may use the fast mode, in which case we don't have a way of knowing which rect changed. So I had to change the KisScanlineFill as explained in the previous point.
    For the enclose and fill I had to change the system from using applicator to stroke directly (as the fill tool does).
    Now, the rect must be shared between the fill and the update commands, so I made it a shared pointer. I also had to change the update command to accept a QSharecPointer<QRect> for this case, because the rect is not ready until the fill visitor command is finished.
  • Change the fill tool to start filling on mouse down instead of up. This is actually not a performance change, but more a ux thing, I would say.
    In the fill tool the fill started on mouse up or on dragging. This in my opinion caused a slight "perceived" lag, maybe some ms, because the users may expect that the fill is initiated on mouse down. So I changed the fill tool to initiate the fill on mouse down.
    Before, if the user dragged the mouse, the drag and fill mode was used, and on mouse up, if there was no dragging, then a normal fill was performed. Now the fill is performed on mouse down, but at this moment we don't know yet if the user is going to drag or not, so we don't know yes if we should use the normal fill or the drag and fill mode. We cannot use always the drag and fill mode because it prevents the fast mode. To solve this, I added a new "do not use" drag and fill option. If it is selected then a normal fill is performed. If one of the other drag and fill options is selected then the drag and fill mode is used.
  • Add optimization in fill tool for when the reference device is empty. This uses a simple check to see if the reference layer used in the fill tool is empty. If it is, then the faster selection fill is used (no flood fill). This can improve performance in the use case where the user creates a new layer and immediately fills it with the fill tool.
  • Improve performance of fill actions by limiting the update rect to the current selection. This adds the same performance improvement related to the update rect to the fill actions. The code is similar to the one on the "drag and drop color to fill" feature.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

Reminder: the reviewer is responsible for merging the patch, this is to ensure at the least two people can build the patch. In case a patch breaks the build, both the author and the reviewer should be contacted to fix the build. If this is not possible, the commits shall be reverted, and a notification with the reasoning and any relevant logs shall be sent to the mailing list, kimageshop@kde.org.

Edited by Deif Lou

Merge request reports