Bug 361448

Summary: Brush strokes flicker when they finish rendering
Product: [Applications] krita Reporter: Sketch Stick <sketchstick>
Component: Instant PreviewAssignee: Krita Bugs <krita-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: andreas, andrew.crouthamel, dimula73, halla, info, nezumi, osxyz, raghu, rjquiralte
Priority: NOR Keywords: investigated, regression, triaged
Version: git master (please specify the git hash!)   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
URL: http://sketchstick.com/files/krita-flicker.webm
Latest Commit: Version Fixed In:
Attachments: Brush example

Description Sketch Stick 2016-04-06 10:41:17 UTC
I'm running Arch Linux, with the latest proprietary NVIDIA drivers.

I've tested numerous opengl, cursor, bit-depth, profile and performance options with no change in behaviour.

Reproducible: Always

Steps to Reproduce:
1. Enable instant preview
2. Make a brush stroke

Actual Results:  
The stroke flickers when it finishes rendering.
Comment 1 Raghavendra kamath 2016-04-06 18:56:41 UTC
I get this behaviour too on 3.0
Comment 2 David REVOY 2016-04-11 13:58:04 UTC
I get this behavior too. 
Note: On my install, the rendering issue totally disappear when using discrete zoom ( eg. 50% ,  33% , etc... ) but I can reproduce the rendering issue when I use a random zoom ( eg. 43%, 36%).
Comment 3 Quiralta 2016-04-15 04:31:46 UTC
Same here, also Archlinux plus Nvidia proprietary driver. Unlike David's zooming doesn't change the behavior though.
Comment 4 Halla Rempt 2016-04-16 12:57:25 UTC
I see the original video is using 32f -- which means uploading the textures while not blocking redisplay is causing artefacts.
Comment 5 Halla Rempt 2016-04-16 13:47:51 UTC
https://phabricator.kde.org/T2145
Comment 6 Dmitry Kazakov 2016-05-14 10:47:47 UTC
Git commit f114bcc1a2251d55cb33395489cc8d52c8f84ee6 by Dmitry Kazakov.
Committed on 14/05/2016 at 10:45.
Pushed by dkazakov into branch 'master'.

Fix flickering in the end of Lod0 recalculation

The problem happened because openGL textures were painted without a
full update. Basically, we shouldn't switch a texture tile from LodN
to Lod0 mode with a partial update, because some part of the tile still
has an old data, causing that weird flickering.

So now we just issue *all* the GUI updates signals in one go, which
basically updates all the tiles and then paints them on screen.

See a comment in KisSuspendProjectionUpdatesStrokeStrategy.

Fixes T2145

M  +17   -6    libs/image/kis_image.cc
M  +3    -0    libs/image/kis_stroke_strategy_factory.h
M  +26   -27   libs/image/kis_strokes_queue.cpp
M  +2    -2    libs/image/kis_strokes_queue.h
M  +121  -22   libs/image/kis_suspend_projection_updates_stroke_strategy.cpp
M  +5    -2    libs/image/kis_suspend_projection_updates_stroke_strategy.h
M  +2    -2    libs/image/kis_update_scheduler.cpp
M  +2    -2    libs/image/kis_update_scheduler.h

http://commits.kde.org/krita/f114bcc1a2251d55cb33395489cc8d52c8f84ee6
Comment 7 wolthera 2017-07-24 21:47:22 UTC
*** Bug 382660 has been marked as a duplicate of this bug. ***
Comment 8 wolthera 2017-07-25 11:53:27 UTC
*** Bug 382660 has been marked as a duplicate of this bug. ***
Comment 9 Halla Rempt 2017-09-28 14:24:40 UTC
*** Bug 363957 has been marked as a duplicate of this bug. ***
Comment 10 Andrew Crouthamel 2018-09-15 14:45:49 UTC
I can not reproduce this issue on 4.2.0, it appears this was fixed with the patch previously posted.

Application Version: 4.2.0
KDE Plasma Version: 5.13.5
KDE Frameworks Version: 5.50.0
Qt Version: 5.11.1
Comment 11 Sketch Stick 2018-09-15 15:45:48 UTC
Confirmed that it's still present in 4.1.1 and the latest nightly appimage.

Grabbed a fresh video: https://sketchstick.com/temp/krita_flickering_4.2.webm
Comment 12 Andrew Crouthamel 2018-09-15 16:06:41 UTC
Thanks for the retest, I can't reproduce on my system which is Intel/Mesa. Maybe it is an Nvidia issue? Anyone with an Nvidia Linux system that could test?
Comment 13 Sketch Stick 2018-09-15 16:17:54 UTC
I'm actually using an AMD RX 480 using the mesa/amdgpu driver.

I usually reproduce with a document set to RGB/Alpha, 32-bit float/channel and sRGB-elle-V2-g10.icc

Using a large document and a smudge brush to make it as slow as possible, with progress reporting enabled it should flicker just after it hits 100%.
Comment 14 Andrew Crouthamel 2018-09-15 16:27:30 UTC
(In reply to Sketch Stick from comment #13)
> I'm actually using an AMD RX 480 using the mesa/amdgpu driver.
> 
> I usually reproduce with a document set to RGB/Alpha, 32-bit float/channel
> and sRGB-elle-V2-g10.icc
> 
> Using a large document and a smudge brush to make it as slow as possible,
> with progress reporting enabled it should flicker just after it hits 100%.

Aha! There we go. Thank you for the details, I can confirm this.

Application Version: 4.2.0 (git master)
KDE Plasma Version: 5.13.5
KDE Frameworks Version: 5.50.0
Qt Version: 5.11.1

Intel HD Graphics 620
Mesa 18.1.8
Comment 15 Dmitry Kazakov 2018-09-18 16:21:20 UTC
The bug actually present on all bit depths (I have tested it). But in 8-bit mode it is happens too quickly to be easily seen :)
Comment 16 Dmitry Kazakov 2018-09-19 08:23:42 UTC
Git commit 58b982cbb7bfa133bbb0bab5b163c24a9bb83d61 by Dmitry Kazakov.
Committed on 19/09/2018 at 08:23.
Pushed by dkazakov into branch 'master'.

Fix flickering on Instant Preview finishing original regeneration

The problem happened mipmap regeneration. Basically, we should not
start regenerating tile mipmaps until all the updates have been
completed. Otherwise, some parts of the regenerated mipmap will
have outdated (old) content and the user will see flickering.

Now KisImage has two special signals that notify the canvas thay
a special "lod rest updates batch" is coming. The canvas will block
tile regeneration for this batch and will start it right after the
batch is completed. Therefore the user will see completely prepared
data.
Fixes T2145
CC:kimageshop@kde.org

M  +20   -0    libs/image/kis_image.h
M  +16   -0    libs/image/kis_image_signal_router.cpp
M  +3    -0    libs/image/kis_image_signal_router.h
M  +19   -5    libs/image/kis_suspend_projection_updates_stroke_strategy.cpp
M  +2    -0    libs/ui/canvas/kis_abstract_canvas_widget.h
M  +70   -16   libs/ui/canvas/kis_canvas2.cpp
M  +2    -0    libs/ui/canvas/kis_canvas2.h
M  +4    -0    libs/ui/canvas/kis_qpainter_canvas.h
M  +23   -0    libs/ui/canvas/kis_update_info.cpp
M  +21   -0    libs/ui/canvas/kis_update_info.h
M  +11   -4    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +1    -0    libs/ui/opengl/kis_opengl_canvas2.h
M  +15   -20   libs/ui/opengl/kis_texture_tile.cpp
M  +8    -4    libs/ui/opengl/kis_texture_tile.h

https://commits.kde.org/krita/58b982cbb7bfa133bbb0bab5b163c24a9bb83d61
Comment 17 Dmitry Kazakov 2018-09-20 17:34:12 UTC
Git commit b5c33311b1f70f3bd249461a31a7333f964e5fb4 by Dmitry Kazakov.
Committed on 20/09/2018 at 17:08.
Pushed by dkazakov into branch 'master'.

Fix delays in the end of Instant Preview rendering stroke

Needs your testing!

Test Plan:

1) Paint with complex brushes on Float32 images
2) Try painting many repetitive strokes, there should be no
   flockering and no delays.
3) Try cancel your srokes before they are finished with Esc key.
   Please note that the most dangerous moment is when Krita writes
   "Updating..." right before background stroke calculation is
   finished. Try pressing exactly at this moment! :)

Technical Details:

There was a short delay in the end of every Instant Preview
regeneration stroke. It was mostly seen on bigger canvases and
Float32-bitdepth mode.

The cause of this delay was the "update resume" stroke that
tried to upload the 100%-zoom image data to the canvas. Sometimes
such uploading could take up to 1.5 seconds, which could interfere
into the painter's workflow.

This patch does multiple things to mitigate this problem:

1) KisSuspendProjectionUpdatesStrokeStrategy is now suspendable
   (again). It means that Krita will suspend the uploading process
   if the user desides to paint further instead of waiting for the
   background rendering to complete. This is the main part of this
   patch.

2) On resuming Krita will not upload the entire image to the canvas,
   but only a changed part. This is achieved by collecting dirty
   requests in KisImage::enableUIUpdates(). This method itself doesn't
   solve the initial problem, but it makes uploading a bit more efficient.

3) While the resume stroke is suspended, KisOpenGLCanvas2 blocks all the
   synchronizations of tiles' mipmaps. It means that normal lodN strokes
   will run with the mipmaps blocked. It is a bit dangerous approach, but
   it works until KisSuspendProjectionUpdatesStrokeStrategy is the only
   user of sigRequestLodPlanesSyncBlocked() signal.

CC:kimageshop@kde.org
Fixes T2145

M  +3    -3    libs/global/kis_pointer_utils.h
M  +1    -0    libs/image/CMakeLists.txt
M  +2    -2    libs/image/KisFakeRunnableStrokeJobsExecutor.cpp
M  +1    -1    libs/image/KisFakeRunnableStrokeJobsExecutor.h
M  +3    -3    libs/image/KisRunnableBasedStrokeStrategy.cpp
M  +1    -1    libs/image/KisRunnableBasedStrokeStrategy.h
M  +2    -2    libs/image/KisRunnableStrokeJobData.cpp
M  +3    -3    libs/image/KisRunnableStrokeJobData.h
C  +4    -11   libs/image/KisRunnableStrokeJobDataBase.cpp [from: libs/image/KisRunnableStrokeJobsInterface.cpp - 070% similarity]
C  +10   -13   libs/image/KisRunnableStrokeJobDataBase.h [from: libs/image/KisRunnableStrokeJobsInterface.h - 058% similarity]
M  +8    -8    libs/image/KisRunnableStrokeJobUtils.h
M  +1    -1    libs/image/KisRunnableStrokeJobsInterface.cpp
M  +9    -3    libs/image/KisRunnableStrokeJobsInterface.h
M  +15   -1    libs/image/kis_image.cc
M  +4    -21   libs/image/kis_image.h
M  +1    -1    libs/image/kis_image_interfaces.h
M  +8    -10   libs/image/kis_image_signal_router.cpp
M  +34   -2    libs/image/kis_image_signal_router.h
M  +0    -20   libs/image/kis_strokes_queue.cpp
M  +292  -86   libs/image/kis_suspend_projection_updates_stroke_strategy.cpp
M  +7    -2    libs/image/kis_suspend_projection_updates_stroke_strategy.h
M  +37   -17   libs/ui/canvas/kis_canvas2.cpp
M  +3    -2    libs/ui/canvas/kis_canvas2.h
M  +5    -5    libs/ui/canvas/kis_update_info.cpp
M  +6    -4    libs/ui/canvas/kis_update_info.h
M  +1    -1    libs/ui/kis_animation_frame_cache.cpp
M  +1    -1    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +3    -3    libs/ui/opengl/kis_opengl_image_textures.cpp
M  +1    -1    libs/ui/opengl/kis_opengl_image_textures.h
M  +3    -2    libs/ui/opengl/kis_texture_tile.cpp
M  +1    -1    libs/ui/opengl/kis_texture_tile.h

https://commits.kde.org/krita/b5c33311b1f70f3bd249461a31a7333f964e5fb4
Comment 18 Andrew Crouthamel 2018-09-20 18:14:45 UTC
Wow, lots of work there! I'll test it out in a few hours when I get home.
Comment 19 Andrew Crouthamel 2018-09-20 21:47:39 UTC
I've built Krita from latest git master, I don't notice the issue as reported. I do notice some of this similar blocky canvas update, but localized to the brush position as I draw. But that may just be my poor graphics card not keeping up.
Comment 20 Halla Rempt 2018-09-24 07:25:03 UTC
Git commit 8739c93870f0ce63633402ceb175cad9362fd4cf by Boudewijn Rempt, on behalf of Dmitry Kazakov.
Committed on 24/09/2018 at 06:57.
Pushed by rempt into branch 'krita/4.1'.

Fix flickering on Instant Preview finishing original regeneration

The problem happened mipmap regeneration. Basically, we should not
start regenerating tile mipmaps until all the updates have been
completed. Otherwise, some parts of the regenerated mipmap will
have outdated (old) content and the user will see flickering.

Now KisImage has two special signals that notify the canvas thay
a special "lod rest updates batch" is coming. The canvas will block
tile regeneration for this batch and will start it right after the
batch is completed. Therefore the user will see completely prepared
data.
Fixes T2145
CC:kimageshop@kde.org

M  +20   -0    libs/image/kis_image.h
M  +16   -0    libs/image/kis_image_signal_router.cpp
M  +3    -0    libs/image/kis_image_signal_router.h
M  +19   -5    libs/image/kis_suspend_projection_updates_stroke_strategy.cpp
M  +2    -0    libs/ui/canvas/kis_abstract_canvas_widget.h
M  +70   -16   libs/ui/canvas/kis_canvas2.cpp
M  +2    -0    libs/ui/canvas/kis_canvas2.h
M  +4    -0    libs/ui/canvas/kis_qpainter_canvas.h
M  +23   -0    libs/ui/canvas/kis_update_info.cpp
M  +21   -0    libs/ui/canvas/kis_update_info.h
M  +11   -4    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +1    -0    libs/ui/opengl/kis_opengl_canvas2.h
M  +15   -20   libs/ui/opengl/kis_texture_tile.cpp
M  +8    -4    libs/ui/opengl/kis_texture_tile.h

https://commits.kde.org/krita/8739c93870f0ce63633402ceb175cad9362fd4cf
Comment 21 Halla Rempt 2018-09-24 07:25:29 UTC
Git commit 61bc0420ae078a0a7896722f25f6b643f3762b6e by Boudewijn Rempt, on behalf of Dmitry Kazakov.
Committed on 24/09/2018 at 07:05.
Pushed by rempt into branch 'krita/4.1'.

Fix delays in the end of Instant Preview rendering stroke

Needs your testing!

Test Plan:

1) Paint with complex brushes on Float32 images
2) Try painting many repetitive strokes, there should be no
   flockering and no delays.
3) Try cancel your srokes before they are finished with Esc key.
   Please note that the most dangerous moment is when Krita writes
   "Updating..." right before background stroke calculation is
   finished. Try pressing exactly at this moment! :)

Technical Details:

There was a short delay in the end of every Instant Preview
regeneration stroke. It was mostly seen on bigger canvases and
Float32-bitdepth mode.

The cause of this delay was the "update resume" stroke that
tried to upload the 100%-zoom image data to the canvas. Sometimes
such uploading could take up to 1.5 seconds, which could interfere
into the painter's workflow.

This patch does multiple things to mitigate this problem:

1) KisSuspendProjectionUpdatesStrokeStrategy is now suspendable
   (again). It means that Krita will suspend the uploading process
   if the user desides to paint further instead of waiting for the
   background rendering to complete. This is the main part of this
   patch.

2) On resuming Krita will not upload the entire image to the canvas,
   but only a changed part. This is achieved by collecting dirty
   requests in KisImage::enableUIUpdates(). This method itself doesn't
   solve the initial problem, but it makes uploading a bit more efficient.

3) While the resume stroke is suspended, KisOpenGLCanvas2 blocks all the
   synchronizations of tiles' mipmaps. It means that normal lodN strokes
   will run with the mipmaps blocked. It is a bit dangerous approach, but
   it works until KisSuspendProjectionUpdatesStrokeStrategy is the only
   user of sigRequestLodPlanesSyncBlocked() signal.

CC:kimageshop@kde.org
Fixes T2145

M  +3    -3    libs/global/kis_pointer_utils.h
M  +1    -0    libs/image/CMakeLists.txt
M  +2    -2    libs/image/KisFakeRunnableStrokeJobsExecutor.cpp
M  +1    -1    libs/image/KisFakeRunnableStrokeJobsExecutor.h
M  +3    -3    libs/image/KisRunnableBasedStrokeStrategy.cpp
M  +1    -1    libs/image/KisRunnableBasedStrokeStrategy.h
M  +2    -2    libs/image/KisRunnableStrokeJobData.cpp
M  +3    -3    libs/image/KisRunnableStrokeJobData.h
C  +4    -11   libs/image/KisRunnableStrokeJobDataBase.cpp [from: libs/image/KisRunnableStrokeJobsInterface.cpp - 070% similarity]
C  +10   -13   libs/image/KisRunnableStrokeJobDataBase.h [from: libs/image/KisRunnableStrokeJobsInterface.h - 058% similarity]
M  +8    -8    libs/image/KisRunnableStrokeJobUtils.h
M  +1    -1    libs/image/KisRunnableStrokeJobsInterface.cpp
M  +9    -3    libs/image/KisRunnableStrokeJobsInterface.h
M  +15   -1    libs/image/kis_image.cc
M  +4    -21   libs/image/kis_image.h
M  +1    -1    libs/image/kis_image_interfaces.h
M  +8    -10   libs/image/kis_image_signal_router.cpp
M  +34   -2    libs/image/kis_image_signal_router.h
M  +0    -20   libs/image/kis_strokes_queue.cpp
M  +292  -86   libs/image/kis_suspend_projection_updates_stroke_strategy.cpp
M  +7    -2    libs/image/kis_suspend_projection_updates_stroke_strategy.h
M  +37   -17   libs/ui/canvas/kis_canvas2.cpp
M  +3    -2    libs/ui/canvas/kis_canvas2.h
M  +5    -5    libs/ui/canvas/kis_update_info.cpp
M  +6    -4    libs/ui/canvas/kis_update_info.h
M  +1    -1    libs/ui/kis_animation_frame_cache.cpp
M  +1    -1    libs/ui/opengl/kis_opengl_canvas2.cpp
M  +3    -3    libs/ui/opengl/kis_opengl_image_textures.cpp
M  +1    -1    libs/ui/opengl/kis_opengl_image_textures.h
M  +3    -2    libs/ui/opengl/kis_texture_tile.cpp
M  +1    -1    libs/ui/opengl/kis_texture_tile.h

https://commits.kde.org/krita/61bc0420ae078a0a7896722f25f6b643f3762b6e
Comment 22 Andrew Crouthamel 2018-09-24 15:41:45 UTC
I just tested with latest master again. I still get the blocky canvas update for the brush. I've attached a short video. It's choppy in the video, but that's just my laptop, reality isn't that slow :). You can at least see what I'm seeing though.
Comment 23 Andrew Crouthamel 2018-09-24 15:42:35 UTC
Created attachment 115207 [details]
Brush example