• Reduced drawing to one single drawText call using newlines
• Got rid of the additional drawText-call to draw a shadow and use an effects filter instead
• The metrics calculation now happens inside drawText, which is what I should have done from the start; it still uses an optimistic approach (i.e. the first draw will nearly always succeed), but it no longer uses a loop, so I hope it's cleaner now

I'll have another look. But you guys forget that the drawing code also has two different offsets that need also be duplicated in the size code and that the size can only work when there's a painter instantiated that knows the current font. It all leads to code duplication in some form or to more complex code that needs to externalize those constants. More, it also makes future changes like adding color to single lines completely impossible. It's premature optimization, it removes all leeway without any gain. Having one additional draw once at the startup of Krita is really not an issue.

With ANGLE, enabling incremental screen updates seems to be slower than without it, but I don't know enough OpenGL and the internals of ANGLE to find out why.

@neviril Could you try out this patch if it also has the "frames lost" problem (i.e. the one from https://phabricator.kde.org/D8804)?

I mean you can calculate the size, resize the pixmap if needed and then paint once., instead of painting an extra time if a resize is needed.

• Fixes a regression I introduced with the last update that broke the checker board mask setup (this might also fix one or two @alvinhochun's flickering checkboard bugs)
• Simplified the checkerboard mask code (eliminated glBlendColor)
• Modified KisCanvas2::resetCanvas so that the change of setting takes place immediately, not only after opening a new canvas

I would've thought you can just compare the equality even without fuzziness.

(1) see comments and my inline remark.
(2) this happens inside KisOpenGLAccumulator::bind, namely with qFuzzyCompare(m_modelMatrix, modelMatrix). This works good enough in practice, although for very small changes this might pose a problem, but I think in practice matrix either stays the same or changes radically (i.e. like > 0.01). I didn't find a convenient observer or callback pattern to detect these changes, which might be even better of course.

Fixes wrapping mode regression (broken due to my columns loop refactoring)
Adds several comments to core parts of the incremental update
Moves drawCheckers() call into drawImage() to have one defined place of call (this is more logical, as drawImage() always draw checkers if it operates in incremental mode, so it was not logical to not have do it in non-incremental mode); the semantics of drawImage() is thus now: draw checkers and image

Indentation fixed and pushed.

@alvinhochun I assume you mean the wrong spacing in libs/ui/forms/wdggeneralsettings.ui? I'll fix that.

Hm, I thought a lot more about this, and now think I understand that this is not a good solution.

Hi @dkazakov, I cannot reproduce this reliably, it happens randomly but fairly often (like once every one or two weeks) on any of my Macs (iMac and MacBooks) while drawing wildly around over the whole canvas (nothing special, i.e. some non-specific 10 px brush on a 4000 x 4000 canvas).

@alvinhochun Not sure I understand your question. The first try in the first run is the calculation of the correct size, as it's not known beforehand. After that, i.e. in later invocations of KisFpsDecoration::drawDecoration, there will be no two tries unless the pixmap size changed.

Oh darn, this looks like copy & paste at its worst on my side. Thanks for finding this. I know, I need to look at https://bugs.kde.org/show_bug.cgi?id=386620.

fix spaces

Hi @rempt, I looked into this today a bit as you mentioned it yesterday and it's indeed quite depressing on macOS.

Solves the alpha background problem by using masking via blend. Works now as far as I can tell. Probably needs more cleaning up inside KisOpenGLCanvas2::drawImage.

After talking to @boud in the meeting, I now understand that the opaqueness thing is not detectable via layer opaqueness at all, as each layer is full RGBA. So the fix for this has to be the "draw one tile background" thing. Ok.

@woltherav says that this is needed on Linux and Windows as well, so I'll update this to get enabled on all platforms

An improved version, but not yet ready for prime time.

pushed.

In D8550#163275, @rempt wrote:

Am I right that I cannot apply this patch at the same time as D8603: Asynchronous texture uploading ?

Fixes the crash on Linux (uninitialized m_sharedContext from one constructor; now implemented differently); and actually any system that has no support for the approach due to lacking threaded OpenGL or shared GL contexts
Better separation of concerns (not done yet)
Less mutex contention
Still very much a work in progress :-)

In D8603#163301, @rempt wrote:

On OSX, I don't see visual regressions, but I get a lot of console messages like

WARNING: Extra data object: 0

Could that be related?

Abandoned, as this needed another inexplicable glFlush to fix regressions that probably only worked because it added another delay. The main problem with this is that glTestObjectAPPLE always returns 1 (done) even directly after uploading a texture. Whereas using glFinish() seems to work, so we want glFinish(), which basically negates the whole approach of this patch.

Ok, I'll abandon this patch, it's not functioning safely. I have another one coming with a threaded approach that's the way to go.

Just for the record, I just found out that this whole approach does not work properly on my MacBook Pro 2017 under High Sierra. Basically, glTestObjectAPPLE returns true (finished) all the time, even though the thing is not finished.

switched to KisSignalCompressor.

I tried to incorporate @dkazakov's suggestions into a simplified state KisSignalCompressor, but it didn't work out as I expected. We still need more state vars, as a compressor can get active and only later on decide if it will emit, but it needs to track its active state independently of its emission state.

rebased to current top of tree

Hi @dkazakov, this looks like a clean design, I'll change the patch accordingly.

Hi @alvinhochun, if you can take a look that would be great, I don't have a Windows test machine to reproduce the effect currently.

pushed.

Ok, I was lacking understanding of QT QObject hierarchies.

Can this still be useful for other OpenGL warnings?

Making the texture size larger is not an option. To address the profiling measurements, the right approach is to use an FBO and only redraw those tiles that need changing.

@dkazakov Ok to push?

@dkazakov Ok to push?

In D8418#159970, @alvinhochun wrote:

It doesn't seem difficult: You only need to get the function pointer once to pass it to KisTextureTile and KisTextureTile is constructed in only one place anyway. It already passes QOpenglFunctions to it so passing an extra function pointer doesn't seem any uglier.

Seems to fix regression.

Actually I detected a regression with this on my iMac 5K right now. When painting with a 100px Ink_brush_25 on a 4000 by 4000 canvas, this happens (notice the grey zebra-like areas in the stroke that actually disappear as soon as an additional update happens in the area):

Adds Q_OS_OSX guard.

This tries to address some of the issues brought up by Dmitry and Alvin.

pushed.

The scrollbar on "On Touch Drag" is a simple change.

pushed.

Reverted.

Hi @dkazakov, I applied the changes, but during final testing I found that the code currently does not free anything, as after changing the texture size to 2048, the freeThreshold of 64 is never reached (the number of allocated tiles is now around 7 to 10 in my tests). Lowering freeThreshold to 8 enables purging again, but this is really a problem related to freeThreshold and texture size, not this PR.

Switched from QTimer to KisSignalCompressor.

@dkazakov Just recognized that this change has implications for the pool sizes defined in kis_texture_tile_info_pool.h:

pushed.

pushed to master.

Fixes performance problem in KisMimeData::retrieveData by not serializing nodes but using internal nodes instead. The functionality (i.e. namely first calling KisMimeData::tryLoadInternalNodes) now lives in the new function KisMimeData::loadNodesFast; it was originally inside KisMimeData::insertMimeLayers and there was no point in copy pasting that code to the place where the drop functionality now needs it.

@alvinhochun Hi Alvin, sorry for the late reply, I didn't see this one. I'm locally still testing using QT 5.9.1.

I like QOpenGLContext::getProcAddress and it works here but it adds some more wrapper code: there needs to be a place to keep the ptr and having it inside KisTextureTile at least needs to pass in the OpenGL context when constructing the tile and then it's kind of ugly, looking up the function for each single tile. And having a whole wrapper like for fences is overkill here. In short, linking against the OpenGL framework IMHO produces more concise code changes here even though generally I'm also a getProcAddress advocator. But if we have Q_OS_OSX defined, we know we're in an ecosystem where that call is always available, and it's also not useful for other platforms. So unless adding an additional linked library is a bad hit which could cause problems (I'm not really knowledgeable about that I admit), I'd prefer the current approach.

pushed to master.

the glSync functionality works for me as expected (e.g. glSync effectiveness: 0.0598802).

Renames version to numFrees which is a more accessible name.

@rempt Is this ready to land now? For some reason, even though you accepted the revision, the item as whole still shows up as "Needs Review".

pushed to master.

Now superseded by D8418.