Add antialiasing to createThumbnailDevice
ClosedPublic

Authored by eingerman on Jun 23 2016, 7:19 AM.

Details

Summary
  1. Added oversampling/antialising to createThumbnailDevice. Interpolation is done using KisTransformWorker with bilinear interpolation. Added parameter that controls oversampling ratio.
  2. Added benchmark for new thumbnail code. Results for 6Kx8K image turned into 640px thumbnail see below. About 2x hit in time for 2x oversampling. Quality with 2x oversampling is much better than no oversampling. 4x oversampling slightly better, but not dramatically. See below.
  3. Changed oversampling for overview widget to 2x.
  4. Fixed caching of oversampled thumbnails.
  5. Fixed up functions calls to createThumbnail.


No Oversampling


2x Oversampling


4x Oversampling

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnail()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnail():

161 msecs per iteration (total: 161, iterations: 1)

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnailCached()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnailCached():

0.000059 msecs per iteration (total: 62, iterations: 1048576)

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQ()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQ():

3,962 msecs per iteration (total: 3,962, iterations: 1)

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample2x()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample2x():

269 msecs per iteration (total: 269, iterations: 1)

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample3x()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample3x():

489 msecs per iteration (total: 489, iterations: 1)

PASS : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample4x()
RESULT : KisThumbnailBenchmark::benchmarkCreateThumbnailHiQcreateThumbOversample4x():

701 msecs per iteration (total: 701, iterations: 1)
Test Plan

Run KisThumbnailBenchmark.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
eingerman updated this revision to Diff 4682.Jun 23 2016, 7:19 AM
eingerman retitled this revision from to Add antialiasing to createThumbnailDevice.
eingerman updated this object.
eingerman edited the test plan for this revision. (Show Details)
eingerman added reviewers: dkazakov, rempt.
eingerman set the repository for this revision to R37 Krita.
dkazakov edited edge metadata.Jun 23 2016, 7:55 AM

Hi, @eingerman!

What exactly "oversampling" is? You first NN the image to a bigger value and then scale it down using a worker, right?

10:47 < eugening> 21px - 0.4ms and 1.1ms.
10:48 < eugening> 6Kx8K down to 21x21 - we might just fill the thumbnail with random colors :) :)

Well, we use exactly that sizes for generating the thumbnails in the layers docker and in (in the future) in the channels docker, so it should be fast enough :)

And it seems like the numbers are ok. You can try to use different oversample values in the KisNodeModel for generation of different kinds of thumbnails :)

I'll try to compile your patch now.

rempt edited edge metadata.Jun 23 2016, 3:00 PM

I built this and it seems fine to me: I think this needs to be pushed to a branch so we can ask our artists on irc to pull the branch and test it in a real life scenario.

Just a question:

libs/image/kis_paint_device.cc
1401

Hi, @eingerman!

Could you tell why this function is needed? Probably, it should be somehow integrated into KisSequentialConstIterator?

dkazakov requested changes to this revision.Jun 24 2016, 4:13 PM
dkazakov edited edge metadata.

Hi, @eingerman!

I have looked and tested your patch. There is a significant problem caused by the patch (well, it is partially present even without the patch, but when with it applied it becomes much more visible.

See this video:
http://nonaynever.ru/pub/overview_delay.mkv

The solution for this is to move the overview's thumbnail generation into the background. To achieve this, you should do the following:

  1. On setCanvas() call in the docker code, create a KisIdleWatcher object that would track when the image becomes idle (that is the user does nothing). See an example in KisDocument::Private::setImageAndInitIdleWatcher()
  1. On getting the signal from the idle watcher, create a specially crafted stroke that would generate a thumbnail in the background in a threaded way and emit the next signal that would update the thumbnail in the overview docker. You should write the stroke yourself, here are the notes:
    1. You can find documentation about the strokes framework here: http://dimula73.narod.ru/krita_strokes/strokes_documentation.html
    2. The example of almost exactly the same stroke: KisSyncLodCacheStrokeStrategy
    3. Make sure that your stroke does not affect any main user processes. To achieve this, don't forget to add the following code to the c-tor:
// don't finish currently running user actions, e.g. Transform Tool
setRequestsOtherStrokesToEnd(false);
// don't clear redo information (we don't save any undo info anyway)
setClearsRedoOnStart(false);
// tell the scheduler that this stroke should be canceled as soon as the user starts any action
setCanForgetAboutMe(forgettable);
  1. Implement partial gneration of the thumbnail to make it 1) multi-threadable; 2) cancellable. When the stroke is cancellable, the scheduler can stop this process when the user starts an action, so there will be no visual disturbance. The function call should write parts of the thumbnail into a common paint device. There is also a tricky part: the alignment of the generated patches should be a multiple of "oversampling" parameter.
/**
 * \p srcRect --- the source rect of the entire thumbnail in pixels of (*this)
 * \p thumbnailRect --- the destination rect of the thumbnail in pixels of (*thumbnailDevice).
 * \p srcPatchRect --- the rect in (*this) pixels that should be downscaled and written to the corresponding rect of the thumbnail device. This rect must be aligned to the border of (oversampling) grid
 */
void KisPaintDevice::generateThumbnailData(const QRect &srcRect, const QRect &thumbnailRect, const QRect &srcPatchRect, KisPaintDevice thumbnailDevice, int oversampling, ...);

Your stroke should be started with a set of jobs, each pointing to a different patch. The cancelling happens in a per-job manner. So the bigger the job is, the longer the user will have to wait untul the background stroke is finished.

  1. You can use a function for splitting a rect into smaller patches in: KritaUtils::splitRectIntoPatches(). Tools for aligning the rect: KisLodTransform::alignRect.

I think our plan should be the following:

  1. You apply to get a KDE contributor's account. Write Boud or me as a promoting person.
  2. Push your patch into a separate branch.
  3. Start the work on implementing a stroke in this separate branch.
  4. When it is ready, we will merge the branch into master.
This revision now requires changes to proceed.Jun 24 2016, 4:13 PM

Just a question:

My mistake. This moveBy function is left over from an alternative implementation that turned out to be no faster. I'll clean it up.

Thank you for the info about moving processing off UI thread. I was actually looking at how to do this for overview docker, channel docker and for histogram docker (which causes the most issues).

I just got the KDE account - I'll make a branch for this.

rempt added a comment.Jun 24 2016, 7:11 PM

Awesome! Good work and looking forward to testing the branch.

I think my branch (thumbnail_improvements) is ready for testing. For full effect you should enable Overview docker, Channel docker and Histogram docker.

rempt added a comment.Jul 20 2016, 8:06 AM

It would be good to also send a mail to kimageshop@kde.org so more people know there's an interesting branch ready for testing.

rempt added a comment.Jul 20 2016, 8:30 AM

I just tested it, and it is looking good, with one caveat: the whole update process only starts once you have made a stroke. So until there's a stroke, the channels docker is empty and the overview docker has a very low-quality thumbnail, so maybe the first update should be kicked off on loading/creating an image?

woltherav added a subscriber: woltherav.EditedJul 21 2016, 11:05 AM

Hi Eugene, I actually managed to get your branch working. (It decided to work magically today...)

  1. Both the histogram and the channels docker don't show anything until you restart Krita with both of them visible. I suspect you have them disabled when they are not showing, which is good, but perhaps see if you can connect to a signal somewhere to show. it's probably what boud says above.
  1. The overview docker does not update it's thumbnail when you resize the docker. I need to make a change on canvas to get the docker looking good when going from a small to a large size.
  1. My instinct would like to see a pop-up of the thumbnail in the channels docker, when I wanted to see a close-up of the thumbnail, similar to the one we have on the layers docker. I am not sure how feasible this is, or even in scope.
  1. The channel docker thumbnail is stretched when it's non-square. I know artists prefer to have the ratio kept. Again, same applies as with 3.
  1. The thumbnails in the channels docker start looking odd as hell if you have a opaque layer serving as the background, and toggle the visibility.

Anyway, it's looking good when it works :)

Boud, Wolthera - thank you for testing! I'll check the cases where thumbnails are not updated.

Boud, initial update of thumbnails is done on setCanvas and after that every time KisIdleWatcher fires a signal. So, the thumbnails should show up when image is open or created.

woltherav added a comment.EditedJul 21 2016, 8:02 PM

One thing to add:

I've been able to make and finish a drawing while using these dockers, however, when making a stroke, waiting for it to show up on the overview docker, and then doing ctrl+z, the overview doesn't update to match the undo.

(You can also see the issue with the transparency in the channels docker. It only does that when the image has transparency)

rempt added a comment.Jul 23 2016, 8:22 AM

Lots of good progress! Here are some more testing notes:

Update after undo now works, and in general there seems to be no performance hit that I can see. We might want to tweak the updating to be a bit quicker, though, it seems to take quite a bit of time to kick in.

The histogram docker still has a problem that it only gets filled in after a stroke is made.

There's something wrong with the channels docker when using a 16 bits/channel floating point image. The channel thumbnails get filled with garbage, even though the layer and overview thumgs are fine.

Hi, @eingerman!

I have found a small issue: Channels and Histogram dockers are not updated until you first paint on them.

  1. Close Channels and Histogram docker.
  2. Close Krita
  3. Open Krita
  4. Open any document
  5. Open Channels docker <--- it is empty until you paint anything
  6. Open Histogram docker <--- it is empty until you paint anything

The Overview docker is not affected. It is updated correctly.

I think I was able to fix all the issues that were reported. Please test the current state of thumbnail_improvement branch.

Could you be so kind to merge master into your branch, because I think I experienced memory leaks, but I wouldn't be able to tell if those are caused by your code or that it's an older bug that is fixed already...

Updated thumbnail_improvements to master branch.

rempt accepted this revision.Jul 26 2016, 7:59 AM
rempt edited edge metadata.

I think it's good to go now!

There is a really funny bug in the Overview docker :) Though it is not a regression of the branch. It is also preset in master. But it might be fun to fix :)

dkazakov accepted this revision.Jul 26 2016, 11:41 AM
dkazakov edited edge metadata.
This revision is now accepted and ready to land.Jul 26 2016, 11:41 AM

Okay, I ran the branch in valgrind and did some doodling(as far as that is possible in valgrind)

This is the relevant output, but I am not sure how to interpret it... :

dkazakov requested changes to this revision.Jul 26 2016, 1:13 PM
dkazakov edited edge metadata.

Wolthera's valgrind output shows updaters leakage in the stroke. So let's mark it as needs fixing now

This revision now requires changes to proceed.Jul 26 2016, 1:13 PM

I think I fixed memory leak reported by Wolthera. I was using QPointer incorrectly.

There is a really funny bug in the Overview docker :) Though it is not a regression of the branch. It is also preset in master. But it might be fun to fix :)

It is funny! That's what happens when image scaling is applied repeatedly to the same image in OverviewWidget::resizeEvent. Fixing is not completely trivial, though.

eingerman updated this revision to Diff 5515.Jul 27 2016, 6:42 AM
eingerman edited edge metadata.

Current diff of thumbnail_improvements vs master.

Ok, I now have a crash on closing a doc but not the program:

Thread 1 (Thread 0x7ffff7f0e8c0 (LWP 3036)):
#0  KisCanvas2::currentImage (this=0x0) at /home/krita/kde/src/krita/libs/ui/canvas/kis_canvas2.cpp:776
#1  0x00007ffff76161fd in KisCanvas2::image() const () at /home/krita/kde/src/krita/libs/ui/canvas/kis_canvas2.cpp:777
#2  0x00007fffa991d84e in OverviewWidget::calculatePreviewSize (this=this@entry=0xd6c35b0) at /home/krita/kde/src/krita/plugins/dockers/overview/overviewwidget.cc:123
#3  0x00007fffa991ea0c in OverviewWidget::generateThumbnail (this=0xd6c35b0) at /home/krita/kde/src/krita/plugins/dockers/overview/overviewwidget.cc:234
#4  0x00007ffff577840e in QMetaObject::activate(QObject*, int, int, void**) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#5  0x00007ffff5777cef in QMetaObject::activate(QObject*, int, int, void**) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#6  0x00007ffff5785068 in QTimer::timerEvent(QTimerEvent*) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#7  0x00007ffff577928b in QObject::event(QEvent*) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#8  0x00007ffff654204c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Widgets.so.5
#9  0x00007ffff6546488 in QApplication::notify(QObject*, QEvent*) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Widgets.so.5

#10 0x00007ffff78d8357 in KisApplication::notify (this=<optimized out>, receiver=0xd6c3930, event=0x7fffffffd190)
    at /home/krita/kde/src/krita/libs/ui/KisApplication.cpp:505
#11 0x00007ffff574ee80 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
---Type <return> to continue, or q <return> to quit---
#12 0x00007ffff579d33e in QTimerInfoList::activateTimers() () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#13 0x00007ffff579d879 in ?? () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#14 0x00007fffee64d127 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007fffee64d380 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x00007fffee64d42c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#17 0x00007ffff579e4e7 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#18 0x00007ffff574cefa in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#19 0x00007ffff5754d9d in QCoreApplication::exec() () from /home/krita/Qt/5.6/gcc_64/lib/libQt5Core.so.5
#20 0x0000000000404bc6 in main (argc=1, argv=<optimized out>) at /home/krita/kde/src/krita/krita/main.cc:230

Sorry for finding so many issues :s

eingerman updated this revision to Diff 5533.Jul 27 2016, 11:23 PM
eingerman edited edge metadata.

Fixed crash when last tab is closed. Thanks to Wolthera for finding it.

woltherav accepted this revision.Jul 27 2016, 11:43 PM
woltherav added a reviewer: woltherav.

Ok, I've been using this branch for a week now with few issues(beyond the fixed leak and the crashes), so if that's the last of it, this can merge.

dkazakov requested changes to this revision.Jul 28 2016, 7:39 AM
dkazakov edited edge metadata.

I still get a crash :(

(gdb) bt
#0  0x00007ffff75da817 in KisCanvas2::image() const () at /home/devel5/kde-src/krita5/libs/ui/canvas/kis_canvas2.cpp:766
#1  0x00007fffb317a52e in ChannelModel::data (this=0x72a36a0, index=..., role=6) at /home/devel5/kde-src/krita5/plugins/dockers/channeldocker/channelmodel.cpp:45
#2  0x00007ffff69eef91 in QModelIndex::data (arole=6, this=0x7fffffffca30) at ../../include/QtCore/../../src/corelib/itemmodels/qabstractitemmodel.h:420
#3  QStyledItemDelegate::initStyleOption (this=0x729da40, option=0x7fffffffc690, index=...) at itemviews/qstyleditemdelegate.cpp:305
#4  0x00007ffff69edfcc in QStyledItemDelegate::paint (this=0x729da40, painter=0x7fffffffc9e0, option=..., index=...) at itemviews/qstyleditemdelegate.cpp:409
#5  0x00007ffff698def3 in QTableViewPrivate::drawCell (this=this@entry=0x729b580, painter=painter@entry=0x7fffffffc9e0, option=..., index=...) at itemviews/qtableview.cpp:922
#6  0x00007ffff6998b04 in QTableView::paintEvent (this=0x7290420, event=<optimized out>) at itemviews/qtableview.cpp:1478
#7  0x00007ffff6747f88 in QWidget::event (this=this@entry=0x7290420, event=event@entry=0x7fffffffd0a0) at kernel/qwidget.cpp:9044
#8  0x00007ffff6846b2e in QFrame::event (this=0x7290420, e=0x7fffffffd0a0) at widgets/qframe.cpp:540
#9  0x00007ffff696c5ab in QAbstractItemView::viewportEvent (this=0x7290420, event=0x7fffffffd0a0) at itemviews/qabstractitemview.cpp:1685
#10 0x00007ffff5e123a2 in QCoreApplicationPrivate::sendThroughObjectEventFilters (this=this@entry=0xc19f10, receiver=receiver@entry=0x72971d0, event=event@entry=0x7fffffffd0a0) at kernel/qcoreapplication.cpp:1072
#11 0x00007ffff670503c in QApplicationPrivate::notify_helper (this=this@entry=0xc19f10, receiver=receiver@entry=0x72971d0, e=e@entry=0x7fffffffd0a0) at kernel/qapplication.cpp:3712
#12 0x00007ffff670a516 in QApplication::notify (this=0x7fffffffe040, receiver=receiver@entry=0x72971d0, e=e@entry=0x7fffffffd0a0) at kernel/qapplication.cpp:3499
#13 0x00007ffff78c9d97 in KisApplication::notify (this=<optimized out>, receiver=0x72971d0, event=0x7fffffffd0a0) at /home/devel5/kde-src/krita5/libs/ui/KisApplication.cpp:505
#14 0x00007ffff5e125bb in QCoreApplication::notifyInternal (this=0x7fffffffe040, receiver=receiver@entry=0x72971d0, event=event@entry=0x7fffffffd0a0) at kernel/qcoreapplication.cpp:965
#15 0x00007ffff6740a79 in QCoreApplication::sendSpontaneousEvent (event=0x7fffffffd0a0, receiver=<optimized out>) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:227
#16 QWidgetPrivate::sendPaintEvent (this=this@entry=0x7297ec0, toBePainted=...) at kernel/qwidget.cpp:5625
#17 0x00007ffff67410c1 in QWidgetPrivate::drawWidget (this=this@entry=0x7297ec0, pdev=0x7355df0, rgn=..., offset=..., flags=36, sharedPainter=sharedPainter@entry=0x0, backingStore=0x7492440) at kernel/qwidget.cpp:5565
#18 0x00007ffff6712856 in QWidgetBackingStore::doSync (this=this@entry=0x7492440) at kernel/qwidgetbackingstore.cpp:1220
#19 0x00007ffff6712a8c in QWidgetBackingStore::sync (this=0x7492440) at kernel/qwidgetbackingstore.cpp:1032
#20 0x00007ffff6730c1f in QWidgetPrivate::syncBackingStore (this=0x6a23730) at kernel/qwidget.cpp:1902
#21 0x00007ffff6747d88 in QWidget::event (this=this@entry=0x6a23520, event=event@entry=0x6e865d0) at kernel/qwidget.cpp:8983
#22 0x00007ffff685dd5b in QMainWindow::event (this=this@entry=0x6a23520, event=event@entry=0x6e865d0) at widgets/qmainwindow.cpp:1495
#23 0x00007ffff6f2642a in KMainWindow::event (this=this@entry=0x6a23520, ev=ev@entry=0x6e865d0) at /home/devel5/kde-src/krita5/libs/widgetutils/xmlgui/kmainwindow.cpp:780
#24 0x00007ffff6f66c19 in KXmlGuiWindow::event (this=0x6a23520, ev=0x6e865d0) at /home/devel5/kde-src/krita5/libs/widgetutils/xmlgui/kxmlguiwindow.cpp:125
#25 0x00007ffff670505c in QApplicationPrivate::notify_helper (this=this@entry=0xc19f10, receiver=receiver@entry=0x6a23520, e=e@entry=0x6e865d0) at kernel/qapplication.cpp:3716
#26 0x00007ffff670a516 in QApplication::notify (this=0x7fffffffe040, receiver=receiver@entry=0x6a23520, e=e@entry=0x6e865d0) at kernel/qapplication.cpp:3499
#27 0x00007ffff78c9d97 in KisApplication::notify (this=<optimized out>, receiver=0x6a23520, event=0x6e865d0) at /home/devel5/kde-src/krita5/libs/ui/KisApplication.cpp:505
#28 0x00007ffff5e125bb in QCoreApplication::notifyInternal (this=0x7fffffffe040, receiver=0x6a23520, event=event@entry=0x6e865d0) at kernel/qcoreapplication.cpp:965
#29 0x00007ffff5e149b6 in QCoreApplication::sendEvent (event=0x6e865d0, receiver=<optimized out>) at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:224
#30 QCoreApplicationPrivate::sendPostedEvents (receiver=receiver@entry=0x0, event_type=event_type@entry=0, data=0xac2950) at kernel/qcoreapplication.cpp:1593
#31 0x00007ffff5e14e98 in QCoreApplication::sendPostedEvents (receiver=receiver@entry=0x0, event_type=event_type@entry=0) at kernel/qcoreapplication.cpp:1451
#32 0x00007ffff5e68643 in postEventSourceDispatch (s=0xd2fc40) at kernel/qeventdispatcher_glib.cpp:271
#33 0x00007fffefba1147 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#34 0x00007fffefba13a0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#35 0x00007fffefba144c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#36 0x00007ffff5e68a4f in QEventDispatcherGlib::processEvents (this=0xd1bda0, flags=...) at kernel/qeventdispatcher_glib.cpp:418
#37 0x00007ffff5e0fd7a in QEventLoop::exec (this=this@entry=0x7fffffffdc80, flags=..., flags@entry=...) at kernel/qeventloop.cpp:204
#38 0x00007ffff5e17e1c in QCoreApplication::exec () at kernel/qcoreapplication.cpp:1229
#39 0x00007ffff614bc3c in QGuiApplication::exec () at kernel/qguiapplication.cpp:1542
#40 0x00007ffff6701495 in QApplication::exec () at kernel/qapplication.cpp:2976
#41 0x0000000000404a66 in main (argc=1, argv=<optimized out>) at /home/devel5/kde-src/krita5/krita/main.cc:230
This revision now requires changes to proceed.Jul 28 2016, 7:39 AM

Here is a video with steps to reproduce:

Should be fixed now.

eingerman updated this revision to Diff 5565.Jul 30 2016, 1:12 AM
eingerman edited edge metadata.

Fixed reported problems.

This revision was automatically updated to reflect the committed changes.