Bug 445561

File imports correctly on Windows (fd399d7a20 (upstream/krita/5.0, krita/5.0) ASL: reasign UUID for resources that share one).

However, the following is logged:

libpng warning: Interlace handling should be turned on when using png_read_image

Does the following patch help?

```
diff --git a/libs/ui/kis_png_converter.cpp b/libs/ui/kis_png_converter.cpp
index 0ec098ff4f..6d6173623b 100644
--- a/libs/ui/kis_png_converter.cpp
+++ b/libs/ui/kis_png_converter.cpp
@@ -490,6 +490,11 @@ KisImportExportErrorCode KisPNGConverter::buildImage(QIODevice* iod)
             (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS))) {
         png_set_expand(png_ptr);
     }
+
+#if defined(PNG_READ_INTERLACING_SUPPORTED)
+    png_set_interlace_handling(png_ptr);
+#endif
+
     png_read_update_info(png_ptr, info_ptr);
 
     // Read information about the png
```

Ok, the patch did not help as libpng is not the issue.

After more testing and getting a kra behave like this I found the problem is that Krita on macos arm (Rosetta or native) 16bit integer colorspace rendering is broken. I remember reading the compiler is messing integer casting, I should check the code and see if this is the issue. Im setting to confirmed.

Created attachment 143677 [details]
Screen cap from latest Krita Stable nightly on Intel macOS

I grabbed the latest Krita Stable nightly (hash in the screen cap) and opened the image successfully on my 2015 MBP.

It seems a Rosetta 2 or (worse) a possible silicon bug.

Now that I recall, https://invent.kde.org/graphics/krita/-/merge_requests/1033 also tinkered with the OpenGL stack on HDR (though on OpenGL ES only).

Created attachment 143678 [details]
Corrupted display on 16bit integer arm macs

This how it loads on macOS M1, Rosetta and arm64 native.

This bug is only present in master and krita 5 beta2. The bug is not present on krita 5 beta1 or krita 4.x series

git bisect shows the bug is introduced by dmitry and my openGL texture buffered patch. It only happens in arm64 apple silicon computers, trough Rosetta and running native. Bisect could not be precise as intermediate states of the MR shown no canvas.

Below is the result of the bisect, I attached a note on the skipped commits.

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
07a10c399b693326f5826217bb0c268aeafb7264       no texture shown (there was something missing dmitry later fixed it)
3104476c49a0fce4ce5ae8eb6a488513b28194db      black texture on 16bit and 32bit
789360ae7ed14f6d91e7e59687a6df8c10eede9d      no texture shown (there was something missing dmitry later fixed it)
92a4f7497fcd95abb3b7fb01cffe4693873604ba        broken textures on 16bit integer
We cannot bisect more!

Created attachment 143754 [details]
broken behaviour

I've found the commit introducing this issue

the commit is part of the openGL buffered refactor. Somehow from commit 07a10c399b693326f5826217bb0c268aeafb7264

commit 07a10c399b693326f5826217bb0c268aeafb7264
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Tue Aug 31 17:14:45 2021 +0300

    Implement a reusable class for multibuffered rendering

    On OSX we have a huge problem with uploading any data on GPU. When we
    upload some data with a buffer and try to write into the same buffer
    again, the writing call is blocked until the previous data has finished
    uploading to the GPU. It makes paintGL() call to block up to 60 ms,
    which stops the main GUI thread and makes OSX to drop tablet events.

 16 bit integer colorspace is rendered wrong in openGL. The files can be saved normally but its impossible to work in this colorspace in a arm macOS (Rosetta or Native). The buffered approach is not the problem as the first buffered "test" commit shows all colorspaces correctly. This bug is not present running from an intel macOS.

I don't think it is an openGL or sillicon bug entirely since the "test" patch works correctly (commit d60aa02cafcc0dd6d9a2881df097847d1a7388d8), I believe there could be a bug at our implementation or maybe in QtOpenGL buffer functions since we didn't use them before.

The video attached shows the wrong rendering changes depending on the moment it get called for refresh, sometimes rendering correctly. An interesting hint to perhaps find where the memory is getting misaligned.

Hi, Ivan!

The related part of the code seems to be in KisTextureTile, in a place where it uses KisOpenGLBufferCircularStorage::BufferBinder to write the data. I don't clearly see how it can affect stuff though.

The test plan is the following:

1) Please check if enabling/disabling "Use texture buffers" in Krita settings makes any difference. Looking at the code, when you disable it (and restart Krita), it should work fine on M1.

2) The rest of the tests should happen with texture buffering **enabled**

3) Disable this part of buffering using this patch: https://invent.kde.org/-/snippets/1967 It should basically switch to the old, unbuffered way of uploading tiles.

4) Revert the previous patch and try to disable buffer invalidation right after upload. Just hardcode `return false` in `KisOpenGL::useTextureBufferInvalidation()`. Though I don't think it should affect anything because it is absent in the commit you bisected.

5) Revert all the previous patches and apply this patch that adds an assert: https://invent.kde.org/-/snippets/1968 It will catch the case if `KisOpenGLBufferCircularStorage` has been created for a different colorspace than the image actually uses. If the assert crashes, then the problem is in `KisOpenGLImageTextures::initBufferStorage()`. It probably gets wrong color space via `m_updateInfoBuilder.destinationColorSpace()` due to some extension detection problem.

1) unchecked texture buffers fixes the rendering problem as suspected (as a quick fix we could force texture buffers off if 16bit integer is used)
3) applying patch and recompiling also fixes the issue (texture buffers is checked but the patch turns it off).
4) Bug is present as expected
5) I've already tested if the buffer and data sizes would differ or not, which did not. The ASSERT as expected does not trigger and printing values shows the size is as expected: lower or equal. 

I tried to find  where there could be a wrong calculation related to the changes from that commit, and could not make it behave differently other than total black, transparent or crashes. Could it be the problem is inside qtopenglbuffer? I mean native arm qt for macos is official in 6.2 version and they are planning on releasing 5.15 somewhere in the future. However I could not find a bug report related.

Hi, Ivan!

More questions:

1) Did you check the patches on master or on the top of the failing commit (07a10c399b693326f5826217bb0c268aeafb7264)? The failing commit had a few other issues, which can cause invalid memory access. These issues were fixed in commits a319898a6f80a1080fc9e056d0d74c2cd4abd3e5 and cd5e1156903f54af811bb7d07141bd4dc1c48de0 

2) Apply this patch, which disables buffering for the tile borders (https://invent.kde.org/-/snippets/1969). The version before the patch didn't have it, it wrote directly into the texture (which is actually a deprecated behavior). The removing this buffering works, then it might be that GPU doesn't like the fact that we use the same buffer for multiple glTexImage2D() calls (which is crazy, because the buffers are created specifically for that).

3) Revert the previous patch. Try making the number of texture buffers fixed. To achieve this, comment out the call to `m_bufferStorage.allocateMoreBuffers(nextSize)` in 'KisOpenGLImageTextures::recalculateCache()' **and** change the initial number in `KisOpenGLImageTextures::initBufferStorage()`

    * set numTextureBuffers to 1. It should make the code "almost" like it used to be before.
    * set numTextureBuffers to 512 (or some other big number). It should catch the case if the M1's driver has troubles with synchronization of the buffer flushes (which is a possible cause, looking at the screenshot).

4) Revert all the previous patches and try to enable buffer flushing by force-returning `true` from `KisOpenGL::useTextureBufferInvalidation()`.

5) If nothing above works, try to check out the last working version of the code (commit d60aa02cafcc0dd6d9a2881df097847d1a7388d8) and check if  `m_useBuffer` is actually true in `KisTextureTile::update()`. Afair, we used to disable buffering on OSX before my patches, so, perhaps, the buffering was just disabled?

...

6) Could you also get a tiny bit of debugging info from `KisOpenGLImageTextures::updateTextureFormat()` using this patch? https://invent.kde.org/-/snippets/1970 It doesn't look as if it could be the cause of the problem, but who knows...

This time I made more indepth testing but the end result is I don't have a clear idea on what this is happening, afair our code is working correctly and the problemas might be from the arm/rosetta layer translation tool deep inside openGL or Qt. This is not a problem present in any krita version running on an intel chip. But its present on arm and Rosetta.

Below are the results of the tests


1) I applied all patches over master when testing :)

2) border buffers makes no difference. I actually tried that yesterday, removing buffers and noted only the inner buffers cause the issue.

3) This is interesting, testing to 1 buffer does not make it behave as it was before, in fact, the texture tiling is broken and still shows the image broken. If we go to 512. the only main difference is that on many buffers the events sometimes does not update the canvas and leaves some parts invisible or not rendered.

4) Buffer flushing is set to only be active on 4.3 versions. macOS openGL latest version is 4.1, this appears to be the reason making the activation of buffer flushing crash krita. So it was not possible to check if there was any change in rendering.

5) tested forcing m_useBuffers and in both tests the image render was normal on any colorspace. The only rendering problem was due to outline compositing and was present in all colorspaces. when there is no pixels between the outline and the checkerboard pattern, and appears to be addressed in another much recent commit.

6)
=== 8bit
2021-12-02 15:17:38.321954-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" tilesDestinationColorSpace = "RGB/Alpha (8-bit integer/channel)" ("RGBA","U8" )
2021-12-02 15:17:38.321975-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.format = 80e1
2021-12-02 15:17:38.321980-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.internalFormat = 8058
2021-12-02 15:17:38.321984-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.type = 1401
2021-12-02 15:17:38.340311-0600 krita[38149:28655458] Still unsignalled after processed 17 tiles
2021-12-02 15:17:38.340376-0600 krita[38149:28655458]     increased number of buffers to 32
=== 16bit int
2021-12-02 15:17:59.044839-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" tilesDestinationColorSpace = "RGB/Alpha (16-bit integer/channel)" ("RGBA","U16" )
2021-12-02 15:17:59.044862-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.format = 80e1
2021-12-02 15:17:59.044867-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.internalFormat = 805b
2021-12-02 15:17:59.044871-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.type = 1403
2021-12-02 15:17:59.175831-0600 krita[38149:28655458] Still unsignalled after processed 17 tiles
2021-12-02 15:17:59.175969-0600 krita[38149:28655458]     increased number of buffers to 32
=== 16bit float
2021-12-02 15:18:16.394816-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" tilesDestinationColorSpace = "RGB/Alpha (16-bit float/channel)" ("RGBA","F16" )
2021-12-02 15:18:16.394842-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.format = 1908
2021-12-02 15:18:16.394847-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.internalFormat = 881a
2021-12-02 15:18:16.394851-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.type = 140b
=== 32bit
2021-12-02 15:18:32.446803-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" tilesDestinationColorSpace = "RGB/Alpha (32-bit float/channel)" ("RGBA","F32" )
2021-12-02 15:18:32.446830-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.format = 1908
2021-12-02 15:18:32.446836-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.internalFormat = 8814
2021-12-02 15:18:32.446840-0600 krita[38149:28655458] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.type = 1406
2021-12-02 15:18:42.817410-0600 krita[38149:28655458] QLayout: Attempting to add QLayout "" to KisShortcutsDialog "", which already has a layout
2021-12-02 15:18:42.861259-0600 krita[38149:28655458] Failed to fetch display info: "current platform doesn't support fetching display information"

============== Comparisson printing result of working hash
2021-12-02 15:34:27.398144-0600 krita[76848:28792392] Entering "KisOpenGLImageTextures::updateTextureFormat()" tilesDestinationColorSpace = "RGB/Alpha (16-bit integer/channel)" ("RGBA","U16" )
2021-12-02 15:34:27.398160-0600 krita[76848:28792392] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.format = 80e1
2021-12-02 15:34:27.398164-0600 krita[76848:28792392] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.internalFormat = 805b
2021-12-02 15:34:27.398168-0600 krita[76848:28792392] Entering "KisOpenGLImageTextures::updateTextureFormat()" m_texturesInfo.type = 1403

Hi, Ivan!

> 3) This is interesting, testing to 1 buffer does not make it behave as it was before, in fact, the texture tiling is broken and still shows the image broken

That sounds interesting... It sounds as if M1's openGl cannot track the dependencies between the buffers usage. That is, it allows writing into a buffer that has not finished painting yet.

Could you do the following test:

1) Set the number of buffers to a fixed value of 1024. To achieve this, comment out the call to `m_bufferStorage.allocateMoreBuffers(nextSize)` in 'KisOpenGLImageTextures::recalculateCache()' **and** change the initial number in `KisOpenGLImageTextures::initBufferStorage()` to 1024.

2) Create **a small** image of 200x200@16i in size (one tile). And try to paint on it for a couple of seconds. It should work for some time and then start failing. Or not start failing at all.

3) Restart Krita. Create an image of 400x400@16i in size (four tiles). And try to paint for a couple of seconds.

The results of these tests can be interpreted as follows:

1) If the you can paint on a single-tile image forever, or if you see artifacts from time to time, but they heal themselves with the next update, then the problem is in race conditions in the openGL pipeline. We write into a buffer that is currently accessed by the GPU. Though looking into your previous screenshots I don't think it is the case.

2) If you can paint for some time, then the artifacts start to appear and become worse and worse with time, then the problem is in the fact that the buffer cannot be reused for the second time. That is, openGL implementation discards this buffer right after the first usage. It could be caused by either a bug in openGL implementation or by the way how Qt uses it.

3) If you cannot paint on these images at all, it means that the problem is in the buffer format itself. Though it shouldn't happen, because the old implementation of the buffers worked fine.

I will make one more test a bit later, but I'll add it as a separate comment.

Hi, Ivan!

Here is one more test for you. Please apply the following patch and try to load the failing image on M1:

https://invent.kde.org/-/snippets/1983

The patch does two things:

1) It adds asserts to all the buffer-related calls. If the calls fails, then it will dump debugging info an crash.

2) It adds asserts to sanity check the current openGL context when creating buffers and when accessing them. That sounds as a quite possible cause of the problem. It might be that the buffers are allocated in a different openGL context, therefore they are inaccessible when we really need. If you happen to trip over any of these asserts, please also check what context is enabled instead of KisOpenGLCnavas2::context() and whether this context belongs to the same QOpenGLContext::shareGroup()  as the canvas' context.

PS:
Btw, could you also check if openGL contexts do actually support sharing and add the following line into the end of KisOpenGLCanvas2::initializeGL():

ENTER_FUNCTION() << this << ppVar(context()->shareGroup()->shares());

There should be at least two contexts, the canvas one and the Qt's one.

Created attachment 144191 [details]
render offset stroke

1) Changed to 1024
2) The small tile does not render for 16i only (however there is something I explain later), all other spaces show the colors and it can be painted. There are no artifacts but the render never shows the image (only checkerboard). After a good amount of strokes painting will start to get shown garbled (as the screenshot from before)
3) Same as before.

== Based on what the canvas is rendering the behaviour is close to number 2) but not quite. I usually test in a single color brush/stroke so it all seemed ok until I started changing colors: On a fresh image supose I make a black stroke, then a red one, then blue, etc.  switching colors on each stroke. The freshly loaded image will not draw anything on canvas (but the thumbnail on layer docker gets updated correctly and on time), however after a certain amount of pixels the stroke you make starts to paint something (broken). From there a long stroke will start cicling all the colors strokes you painted in order. As if there was a misalign memory read only on 16i texture tiles that causes the pixels to be rendered a lot later. There must be some place the wrong format is set when we ask for 16i from the arm compiler.

I attach another image. here I create a 200x200 16i image. the image will not show until I refresh the view many times (changing layer visibility i.e.) Once the gradient is shown I paint a stroke, it will show as a garbled mess at the coordinates of the stroke, but after refresh the new pixels get moved to the top, clearly misaligned (attached image, left garbled, right after many refresh). Refreshing the image will change the particular order of those pixels until enough refresh is done and the stroke gets rendered correctly. This does not happen if texture buffer is unticked. This fixed state only happens in images/canvas updates below 449 by 449, any canvas update larger than that will never be shown properly.

== about the patch
I painted and used the master with the patch loading different 16i images. On loading the image or painting, there are no asserts.

Entering "KisOpenGLCanvas2::initializeGL()" KisOpenGLCanvas2(0x600003519810) context()->shareGroup()->shares() = (QOpenGLContext(0x600000009dc0, nativeHandle=QVariant(QCocoaNativeContext, ), format=QSurfaceFormat(version 4.1, options QFlags<QSurfaceFormat::FormatOption>(), depthBufferSize 32, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior QSurfaceFormat::DoubleBuffer, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile  QSurfaceFormat::CoreProfile), screen=""), QOpenGLContext(0x6000000263d0, nativeHandle=QVariant(QCocoaNativeContext, ), format=QSurfaceFormat(version 4.1, options QFlags<QSurfaceFormat::FormatOption>(), depthBufferSize 32, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior QSurfaceFormat::DoubleBuffer, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile  QSurfaceFormat::CoreProfile), screen=""), QOpenGLContext(0x600000026160, nativeHandle=QVariant(QCocoaNativeContext, ), format=QSurfaceFormat(version 4.1, options QFlags<QSurfaceFormat::FormatOption>(), depthBufferSize 32, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior QSurfaceFormat::DoubleBuffer, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile  QSurfaceFormat::CoreProfile), surface=0x6000026be530, screen=""), QOpenGLContext(0x60000008e290, nativeHandle=QVariant(QCocoaNativeContext, ), format=QSurfaceFormat(version 4.1, options QFlags<QSurfaceFormat::FormatOption>(), depthBufferSize 32, redBufferSize 8, greenBufferSize 8, blueBufferSize 8, alphaBufferSize 8, stencilBufferSize 8, samples 0, swapBehavior QSurfaceFormat::DoubleBuffer, swapInterval 0, colorSpace QSurfaceFormat::DefaultColorSpace, profile  QSurfaceFormat::CoreProfile), surface=0x600001021b70, screen=""))



Any colorspace switch (image -> colorspace)
ASSERT (krita): "m_textureTiles.size() > tile" in file /Users/daedalus/developer/krita/repos/master/krita/libs/ui/opengl/kis_opengl_image_textures.h, line 127

Created attachment 144192 [details]
offset just at right amount

The offset from 450x450 image size.the idea was to try and understand how the offset is generated, this of course is not the general behaviour just a corner case that happens to fall into this pattern. Bigger images garbage output can be periodic depending on the number of tiles and the size of the update canvas region. Smaller updates on bigger images (i.e. layer with single stroke) render completely normal after some cicles.

The problem still seems to be around in the final 5.0 version published just now. I still have the problem with the output which means I can't use 16bit integer, which is unfortunately the depth I am using all the time.
Any way to work around this?!

> Any way to work around this?!

Either working on 16bit Float, or disabling Texture Buffering (Preferences -> Display -> Uncheck "Use Texture buffer")

A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/1259