Differential D8603

Asynchronous texture uploading
Needs RevisionPublic
Actions

Authored by poke1024 on Nov 1 2017, 8:20 PM.

Details

Reviewers
dkazakov
rempt
Group Reviewers
Krita
Summary

Follow up of D8418.

Optionally moves the whole texture upload into a separate thread using a shared GL context. On OS X this allows to use glFinish() after each texture upload without having to block the main thread.

This is still something I'm experimenting on and testing on, nothing final. But still, it's working nicely and looks very promising so far - no regressions. It even works under corrosive test conditions (i.e. using Giphy which breaks the current top of tree).

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
poke1024 created this revision.Nov 1 2017, 8:20 PM
rempt added a subscriber: rempt.Nov 2 2017, 8:34 AM

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?

rempt added a comment.Nov 2 2017, 9:03 AM

On linux, instead, I get a crash:

0 0x00007fffed6e9cad in QObject::moveToThread(QThread*) () at /usr/lib64/libQt5Core.so.5
#1 0x00007ffff776fbe9 in KisOpenGLTextureLoader::KisOpenGLTextureLoader(QOpenGLContext*, QSurface*) (this=

0x16f4af00, context=0x7fff00000001, surface=<optimized out>)
at /home/boud/dev/krita/libs/ui/opengl/kis_opengl_texture_loader.cpp:45

#2 0x00007ffff77638b6 in KisOpenGLImageTextures::createTextureLoader() (this=this@entry=0x84d91b0)

at /home/boud/dev/krita/libs/ui/opengl/kis_opengl_image_textures.cpp:295

#3 0x00007ffff7767367 in KisOpenGLImageTextures::createImageTextureTiles() (this=this@entry=0x84d91b0)

at /home/boud/dev/krita/libs/ui/opengl/kis_opengl_image_textures.cpp:275

#4 0x00007ffff776a542 in KisOpenGLImageTextures::initGL(QOpenGLContext*) (this=0x84d91b0, context=<optimized out>) at /home/boud/dev/krita/libs/ui/opengl/kis_opengl_image_textures.cpp:143
#5 0x00007ffff7761a25 in KisOpenGLCanvas2::initializeGL() (this=0x84d8950)

at /home/boud/dev/krita/libs/ui/opengl/kis_opengl_canvas2.cpp:253

#6 0x00007fffee64d3f2 in QOpenGLWidgetPrivate::initialize() () at /usr/lib64/libQt5Widgets.so.5
#7 0x00007fffee64d58a in QOpenGLWidget::resizeEvent(QResizeEvent*) () at /usr/lib64/libQt5Widgets.so.5
#8 0x00007fffee630321 in QWidget::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#9 0x00007fffee5f4cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#10 0x00007fffee5fba10 in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#11 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0x84d8950, event=0x7fffffffa9f0) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#12 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#13 0x00007fffee6293d2 in QWidgetPrivate::sendPendingMoveAndResizeEvents(bool, bool) ()

at /usr/lib64/libQt5Widgets.so.5

#14 0x00007fffee62cc43 in QWidgetPrivate::show_helper() () at /usr/lib64/libQt5Widgets.so.5
#15 0x00007fffee62cb8f in QWidgetPrivate::showChildren(bool) () at /usr/lib64/libQt5Widgets.so.5
#16 0x00007fffee62cc5f in QWidgetPrivate::show_helper() () at /usr/lib64/libQt5Widgets.so.5
#17 0x00007fffee62f897 in QWidget::setVisible(bool) () at /usr/lib64/libQt5Widgets.so.5
#18 0x00007fffee62cbf0 in QWidgetPrivate::showChildren(bool) () at /usr/lib64/libQt5Widgets.so.5
#19 0x00007fffee62cc5f in QWidgetPrivate::show_helper() () at /usr/lib64/libQt5Widgets.so.5
#20 0x00007fffee62f897 in QWidget::setVisible(bool) () at /usr/lib64/libQt5Widgets.so.5
#21 0x00007fffee62cbf0 in QWidgetPrivate::showChildren(bool) () at /usr/lib64/libQt5Widgets.so.5
#22 0x00007fffee62cc5f in QWidgetPrivate::show_helper() () at /usr/lib64/libQt5Widgets.so.5
#23 0x00007fffee62f897 in QWidget::setVisible(bool) () at /usr/lib64/libQt5Widgets.so.5
#24 0x00007fffee62cbf0 in QWidgetPrivate::showChildren(bool) () at /usr/lib64/libQt5Widgets.so.5
#25 0x00007fffee62cc5f in QWidgetPrivate::show_helper() () at /usr/lib64/libQt5Widgets.so.5
#26 0x00007fffee62f897 in QWidget::setVisible(bool) () at /usr/lib64/libQt5Widgets.so.5
#27 0x00007fffee75c0f5 in QMdiSubWindow::changeEvent(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#28 0x00007fffee62fd40 in QWidget::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#29 0x00007fffee75c95b in QMdiSubWindow::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#30 0x00007fffee5f4cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#31 0x00007fffee5fba10 in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#32 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0xa485880, event=0x7fffffffb340) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#33 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#34 0x00007fffee62c318 in QWidget::setWindowState(QFlags<Qt::WindowState>) () at /usr/lib64/libQt5Widgets.so.5
#35 0x00007fffee62c9e1 in QWidget::showMaximized() () at /usr/lib64/libQt5Widgets.so.5
#36 0x00007fffee75c74a in QMdiSubWindow::eventFilter(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#37 0x00007fffed6b8bf1 in QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) ()

at /usr/lib64/libQt5Core.so.5

#38 0x00007fffee5f4c95 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#39 0x00007fffee5fba10 in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#40 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0xa447bb0, event=0x7fffffffb700) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#41 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#42 0x00007fffee62c318 in QWidget::setWindowState(QFlags<Qt::WindowState>) () at /usr/lib64/libQt5Widgets.so.5
#43 0x00007fffee62c9e1 in QWidget::showMaximized() () at /usr/lib64/libQt5Widgets.so.5
#44 0x00007ffff78cf9a5 in KisMainWindow::showView(KisView*) (this=0x7b969b0, imageView=0xa447bb0)

at /home/boud/dev/krita/libs/ui/KisMainWindow.cpp:598

#45 0x00007ffff78cca3c in KisMainWindow::addView(KisView*) (this=this@entry=0x7b969b0, view=view@entry=0xa447bb0) at /home/boud/dev/krita/libs/ui/KisMainWindow.cpp:538
#46 0x00007ffff78ccb32 in KisMainWindow::addViewAndNotifyLoadingCompleted(KisDocument*) (this=0x7b969b0, document=<optimized out>) at /home/boud/dev/krita/libs/ui/KisMainWindow.cpp:864
---Type <return> to continue, or q <return> to quit---
#47 0x00007ffff78e5818 in KisPart::startCustomDocument(KisDocument*) (this=<optimized out>, doc=<optimized out>)

at /home/boud/dev/krita/libs/ui/KisPart.cpp:469

#48 0x00007ffff796d15c in KisPart::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=<optimized out>, _c=<optimized out>, _id=<optimized out>, _a=<optimized out>)

at /home/boud/dev/b-krita/libs/ui/moc_KisPart.cpp:142

#49 0x00007fffed6e21df in QMetaObject::activate(QObject*, int, int, void**) () at /usr/lib64/libQt5Core.so.5
#50 0x00007ffff795d7ef in KisOpenPane::documentSelected(KisDocument*) (this=<optimized out>, _t1=0x7d4f640)

at /home/boud/dev/b-krita/libs/ui/moc_KisOpenPane.cpp:238

#51 0x00007fffed6e21df in QMetaObject::activate(QObject*, int, int, void**) () at /usr/lib64/libQt5Core.so.5
#52 0x00007fffee6e4ce2 in QAbstractButton::clicked(bool) () at /usr/lib64/libQt5Widgets.so.5
#53 0x00007fffee6e4eea in QAbstractButtonPrivate::emitClicked() () at /usr/lib64/libQt5Widgets.so.5
#54 0x00007fffee6e5a9d in QAbstractButton::click() () at /usr/lib64/libQt5Widgets.so.5
#55 0x00007fffee63007a in QWidget::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#56 0x00007fffee5f4cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#57 0x00007fffee5fb94c in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#58 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0xa30d800, event=0x7fffffffc230) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#59 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#60 0x00007fffee64b26b in QWidgetWindow::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#61 0x00007fffee5f4cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#62 0x00007fffee5fba10 in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#63 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0xa3c0510, event=0x7fffffffc230) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#64 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#65 0x00007fffedc799d0 in QGuiApplicationPrivate::processKeyEvent(QWindowSystemInterfacePrivate::KeyEvent*) ()

at /usr/lib64/libQt5Gui.so.5

#66 0x00007fffedc7e3a5 in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) () at /usr/lib64/libQt5Gui.so.5
#67 0x00007fffedc5c8ab in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib64/libQt5Gui.so.5
#68 0x00007fffe1b365b0 in userEventSourceDispatch(_GSource*, int (*)(void*), void*) ()

at /usr/lib64/libQt5XcbQpa.so.5

#69 0x00007fffe8aac134 in g_main_context_dispatch () at /usr/lib64/libglib-2.0.so.0
#70 0x00007fffe8aac388 in () at /usr/lib64/libglib-2.0.so.0
#71 0x00007fffe8aac42c in g_main_context_iteration () at /usr/lib64/libglib-2.0.so.0
#72 0x00007fffed709dbc in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()

at /usr/lib64/libQt5Core.so.5

#73 0x00007fffed6b701b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()

at /usr/lib64/libQt5Core.so.5

#74 0x00007fffee7cd76f in QDialog::exec() () at /usr/lib64/libQt5Widgets.so.5
#75 0x00007ffff78cb46f in KisMainWindow::slotFileNew() (this=<optimized out>)

at /home/boud/dev/krita/libs/ui/KisMainWindow.cpp:1403

#76 0x00007ffff78dc2f5 in KisMainWindow::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x7b969b0, _c=<optimized out>, _id=<optimized out>, _a=0x7fffffffc860)

at /home/boud/dev/b-krita/libs/ui/moc_KisMainWindow.cpp:295

#77 0x00007fffed6e21df in QMetaObject::activate(QObject*, int, int, void**) () at /usr/lib64/libQt5Core.so.5
#78 0x00007fffee5ee8f2 in QAction::triggered(bool) () at /usr/lib64/libQt5Widgets.so.5
#79 0x00007fffee5f105d in QAction::activate(QAction::ActionEvent) () at /usr/lib64/libQt5Widgets.so.5
#80 0x00007fffee5f1224 in QAction::event(QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#81 0x00007fffee5f4cbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()

at /usr/lib64/libQt5Widgets.so.5

#82 0x00007fffee5fba10 in QApplication::notify(QObject*, QEvent*) () at /usr/lib64/libQt5Widgets.so.5
#83 0x00007ffff789eb37 in KisApplication::notify(QObject*, QEvent*) (this=<optimized out>, receiver=0x9d6f1b0, event=0x7fffffffcb90) at /home/boud/dev/krita/libs/ui/KisApplication.cpp:599
#84 0x00007fffed6b8d25 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib64/libQt5Core.so.5
#85 0x00007fffedca85ff in QShortcutMap::dispatchEvent(QKeyEvent*) () at /usr/lib64/libQt5Gui.so.5
#86 0x00007fffedca86c4 in QShortcutMap::tryShortcut(QKeyEvent*) () at /usr/lib64/libQt5Gui.so.5
#87 0x00007fffedc60188 in QWindowSystemInterface::handleShortcutEvent(QWindow*, unsigned long, int, QFlags<Qt::KeyboardModifier>, unsigned int, unsigned int, unsigned int, QString const&, bool, unsigned short) ()

at /usr/lib64/libQt5Gui.so.5

#88 0x00007fffedc79a17 in QGuiApplicationPrivate::processKeyEvent(QWindowSystemInterfacePrivate::KeyEvent*) ()

at /usr/lib64/libQt5Gui.so.5

---Type <return> to continue, or q <return> to quit---
#89 0x00007fffedc7e3a5 in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) () at /usr/lib64/libQt5Gui.so.5
#90 0x00007fffedc5c8ab in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib64/libQt5Gui.so.5
#91 0x00007fffe1b365b0 in userEventSourceDispatch(_GSource*, int (*)(void*), void*) ()

at /usr/lib64/libQt5XcbQpa.so.5

#92 0x00007fffe8aac134 in g_main_context_dispatch () at /usr/lib64/libglib-2.0.so.0
#93 0x00007fffe8aac388 in () at /usr/lib64/libglib-2.0.so.0
#94 0x00007fffe8aac42c in g_main_context_iteration () at /usr/lib64/libglib-2.0.so.0
#95 0x00007fffed709dbc in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()

at /usr/lib64/libQt5Core.so.5

#96 0x00007fffed6b701b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()

at /usr/lib64/libQt5Core.so.5

#97 0x00007fffed6bf5e4 in QCoreApplication::exec() () at /usr/lib64/libQt5Core.so.5
#98 0x000000000040599d in main(int, char**) (argc=1, argv=<optimized out>)

at /home/boud/dev/krita/krita/main.cc:305
dkazakov requested changes to this revision.Nov 2 2017, 9:27 AM
dkazakov added a subscriber: dkazakov.

Yep, confirm. I have the same crash on Linux.

Btw, is it possible to encapsulate this feature into something like you did for KisRelaxedTimer? I mean, it would be useful to be able to enable/disable asynchronous uploading by commenting/uncommenting a couple of lines. It will help debugging the problems in the future a lot :)

The point is, commenting out/disabling all the optimizations is the only way to track down the problems like this: d93da94864a97cdcd1821ed52610517f4e115035

This revision now requires changes to proceed.Nov 2 2017, 9:27 AM
poke1024 added a comment.Nov 2 2017, 6:58 PM
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?

Maybe, but I'm not sure how. Seems to be something in the KisPaintDevice. I don't see it on my two machines here. Might have to do with updates getting delayed.

poke1024 updated this revision to Diff 21817.Nov 2 2017, 11:53 PM

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 :-)

rempt accepted this revision.Nov 8 2017, 9:22 AM

No crash anymore!

dkazakov requested changes to this revision.Nov 14 2017, 1:36 PM

Hi, @poke1024!

Codewise the patch looks very good (except a small unbalanced ifdef, see inline). I do really like how the code is separated from the main rendering pipeline.

There is a weird thing though. I have tested both uploading methods here on Linux and it seems like the asynchronous version is significantly *slower* that the sync one. I can ever see it with my eyes :(

Can you check why is it so slow?

Testcase:

  1. Disable instant preview
  2. Activate openGL and brush FPS debugging in the Performance tab
  3. Create a 8000x6000 image
  4. Activate Brish_tip_default brush and resize it to be 1000px is size.
  5. Start doing a very long stroke, moving your stylus/mouse as fast as possible.
  6. Check the log for "Requested FPS" number. That is the FPS that the user sees on the screen.

Here are my results:

# with async texture loading

Requested FPS: 1.636
Requested FPS: 38.6529
glSync effectiveness: 0.658683
Requested FPS: 29.5927
glSync effectiveness: 0.680639
Requested FPS: 14.5912            ### <---- see these numbers!!! 
glSync effectiveness: 0.738523
Requested FPS: 17.9205
Requested FPS: 12.5763
glSync effectiveness: 0.758483
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" isSamePreset = true
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" " CS: 83.5634  RS: 11.7293  FPS: 8.45808 (saturated)"
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" "ACS: 56.9023 ARS: 11.3622 AFPS: 9.93757"


# without async texture loading

Requested FPS: 13.1203
Requested FPS: 39.8265
glSync effectiveness: 0.413174
Requested FPS: 31.6614
Requested FPS: 27.7701
Requested FPS: 26.614
Requested FPS: 27.6864
glSync effectiveness: 0.225549
Requested FPS: 25.2122
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" isSamePreset = false
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" " CS: 70.6147  RS: 11.5984  FPS: 8.93369 (saturated)"
Entering "KisStrokeSpeedMonitor::notifyStrokeFinished()" "ACS: 70.6147 ARS: 11.5984 AFPS: 8.93369"
libs/ui/opengl/kis_opengl_canvas2.cpp
374

It really looks like there is an unbalanced pause/resume on OSX... Why is it needed to disable resuming on OSX?

If it is really needed, could you move this OSX check into the loader itself? It could be a special method named like resumeSafeWorkaroundXXX() if needed :) Right now it is impossible to guess why this ifdef is here :)

This revision now requires changes to proceed.Nov 14 2017, 1:36 PM

Revision Contents
Changeset List

Diff 21817

View Options

libs/ui/CMakeLists.txt

Loading...
View Options

libs/ui/kis_config.h

Loading...
View Options

libs/ui/kis_config.cc

Loading...
View Options

libs/ui/opengl/kis_opengl_canvas2.h

Loading...
View Options

libs/ui/opengl/kis_opengl_canvas2.cpp

Loading...
View Options

libs/ui/opengl/kis_opengl_image_textures.h

Loading...
View Options

libs/ui/opengl/kis_opengl_image_textures.cpp

Loading...
View Options

libs/ui/opengl/kis_opengl_texture_loader.h

Loading...
View Options

libs/ui/opengl/kis_opengl_texture_loader.cpp

Loading...
View Options

libs/ui/opengl/kis_texture_tile.h

Loading...
View Options

libs/ui/opengl/kis_texture_tile.cpp

Loading...