Tuesday, 28 June 2016

The Road of Trials

Hello all!

In my last blog post I said that I would work on extending support for paint operations like 'fill'. I have done so, albeit more as a necessity in fixing the assistant code. Moreover, I have fixed a number of other paint operations which are vital in painting the various assistants Krita offers currently.

Tool Outline

Before I talk about the assistant fixes I would like to talk about my fix of the tool outline code. This code is responsible for drawing the brush outline which follows the cursor and some of the selection previews while the user is dragging the selection over the canvas. Prior to starting work on the porting of our decoration code to OpenGL 3.2 some work had been done already by another member of the Krita community (beelzy). She changed several deprecated drawing functions to make use a vertex array object and multiple vertex buffer objects. The idea was that we bind a single VAO and instead of uploading the data directly to the GPU, the data is now first uploaded to a vertex buffer object which can be used for drawing the shape.

This approach works fine for drawing the canvas on the screen and drawing the chequerboard texture you see if your layer is transparent, however it broke down for drawing the tool outline. The reason for this is that the tool outline is.. well.. a line! So as opposed to drawing a quadrilateral polygon (rectangle) for the canvas we now want to render a line. This isn't a problem on its own however we can see that the data required to draw a 'quad' is significantly different than for a line. Besides, the data required to draw a selection as it is being dragged over the canvas changes significantly in a short period of time whereas the data requires to draw the canvas shape remains the same for longer periods of time.
Wouldn't it be nice if we could have one buffer for storing the data required to draw the canvas shape, and another buffer for storing the data required to draw the volatile lines?

To address this issue I split our original vertex array object (VAO) into two of these objects. One specifically meant for drawing quadrilaterals, and the other specifically for drawing lines. On top of that, I specified in these objects that the quad data doesn't change very much by setting it to GL_STATIC_DRAW and that the line object does change a lot by setting it to GL_STREAM_DRAW. The OpenGL documentation explains these constants better than I can so I will just post them here.

GL_STATIC_DRAW: The data store contents will be modified once and used many times.
GL_STREAM_DRAW:  The data store contents will be modified once and used at most a few times.

So that is just what we want! We define the data required to draw a rectangle once and render it many times. In contrast, we keep redefining the data required to draw the line at that particular moment and draw it at most a few times.

Now the astute reader might note that the canvas size and shape might change as well during zooming or rotating. And that's true! Here is a little dilemma we have to answer. The traditional way of handling these transformation is by using a matrix. The matrix contains all the transformation necessary so that when it is applied to every vertex of the polygon the whole polygon is transformed to its new location and shape in the Vertex Shader. This doesn't change the vertex positions stored in the graphics card, but rather puts them in the correct position every frame. A more naive approach might be to just re-upload the already transformed vertices to the graphics card. The reason why I say naive is that this approach would obviously cause huge amount of slowdown when we are working with a lot of vertices (like a game model). Uploading tens of thousands of vertices as opposed to a matrix consisting of 16 floating point numbers every time a model changes position would be stupid. However, here we are only working with 4 vertices. In this case we would only have to re-upload 8 floating point numbers to the graphics card in order to update the shape. Moreover, maybe we don't even have to re-upload the data on every frame but only when the user changes the size or position of the canvas. Turns out this approach is not so naive and may be the best option.

The metric 12 fiasco

For the past few weeks my terminal has been spammed by a single warning over and over again. "QOpenGLWidget::metric(): unknown metric 12". So this week I decided to go out and investigate what was causing this so that I could debug in peace. After tedious grepping and taking notes I traced it back to metric 12 coming from somewhere inside QWidget. So now that I knew where metric 12 was coming from and what it was supposed to do imagine my surprise when I went into QOpenGLWidget and found that the handling of metric 12 had simply been commented out. D'oh! Turns out that during the beta phase of Qt 5.6.0 they commented out this handling and our forked paint engine was based on this beta version. So I updated my Qt to the new version 5.6.1 and copied its handling of metric 12 back into our paint engine. Let's hope there aren't more surprise changes between our forked paint engine and Qt 5.6.1's paint engine.

Perspective assistant

In my previous post I noted that many assistants crashed upon the first click on the canvas. For some profane reason I decided to tackle the perspective assistant first, and boy did I regret that.

Here is a partial compendium of all the different things the perspective assistant consists of:

  • Corner nodes
  • Nodes between corner nodes
  • Handles for nodes between corner nodes
  • Grid between nodes
  • Shapes behind the icons of the assistant widget
  • Icons on the assistant widget
  • Lines while moving nodes
  • Lines drawn in hidden mode
  • Crosshair where perspective lines meet
So where to even begin? Well the way I started was putting down a return statement at the start of every perspective function I could find (and there are quite a few). Then by removing these statements one by one I would find it to crash multiple times. Each of these crashes relates to a path ultimately leading to a function in the paint engine that was still using legacy code.

One such path lead to a new branch in the code that is responsible for painting strokes (aptly named stroke()). This new branch involved code for painting non-opaque strokes. Qt does this by using a stencil buffer (for reasons which are unclear to me). This branch seems to be called for the drawing of the grid lines which are (barely) transparent.

Another path was responsible for drawing the assistant widget which consists of a circle and a rounded rectangle combined to form the background and 3 icons pasted on top of it. The icons are drawn using QPainter::drawPixmap, which calls drawImage, which calls drawTexture, which calls.. wait what's another word for image?

And then the circle.. ultimately it gets drawn using a function called drawVertexArrays() in the paint engine. This function receives all the vertices of the circle. But then how do you fill that circle? Well there are many ways, however since I saw that it was being drawn using GL_TRIANGLE_FAN which makes a fan of triangle I assumed the following shape:

The first vertex in the array would be taken as the central hub and all consequent vertices would form a triangle with the hub and the previous vertex. But there's a problem, the data I received in the function didn't contain a central vertex. How does this work? Well turns out it works pretty cleverly by just taking one of the circle vertices as the central hub. It results in a circle that looks like this in wire-frame:

I should note at this point that the assistant code is very old and quite messy which resulted in a lot of confusion over which code was responsible for what. I will probably visit it again at some point to clean it up and hopefully make the assistant faster.

Some more bugs caused by my own stupidity kept me busy for the remainder of the time this week, but I won't bother you with the details (I'm ashamed).

Previously I made a graph to keep track of which decorations were broken and which were fixed. Well... turns out I had less merit from that than I expected. The perspective assistant was hoarding all the methods required to draw every assistant. And since I fixed the perspective assistant, by proxy I fixed every other assistant that we offer. In addition, I fixed the tool outline painting which was also used by many selections, so this is the status now:

I expected some assistants to be fixed, but I certainly didn't expect this. In any case there are still a lot of code paths in the paint engine which aren't called by Krita but still need to be ported to OpenGL 3.2.

Finally, the way that I currently fix the paint engine code is by uploading the data to a VBO instead of directly passing it to the graphics card (which isn't allowed anymore). One can imagine however that adding this extra step in between doesn't mean the code becomes faster. The way in which OpenGL 3.2 could be faster is by uploading our data once and then drawing it multiple times. So over the coming weeks I will investigate if it is possible to cache certain drawing operations so that drawing the same thing over and over again doesn't need any uploading of data to the graphics card. That is where a speed up will come from and that is what Qt would be happy with.

Oh, and here's a little picture of all the assistants in action:


  1. Fantastic progress. You are also quite a good writer. Keep it up!

  2. Good job so far !
    Very interesting to read tyour reports