Differential D12972

OpenEXR Loading Performance Improvements
ClosedPublic
Actions

Authored by jhoolmans on May 18 2018, 9:29 PM.

Details

Reviewers
rempt
Group Reviewers
Krita
Commits
R37:e649485bf712: OpenEXR loading performance optimizations.
R37:c94a09030c86: OpenEXR loading performance optimizations.
Summary

This patch will improve loading time for bigger exr files. Recorded speed improvements with about 5 times.

Some other fixes are included with this patch also.

  • Loading cropped exr files with different display window and data window. The image will now have the size of the displayWindow.
  • Fixed crash when saving 8-bit integer images to .exr and flattening the image.
  • Changed when the non-zero color and non-zero alpha warning shows up. I've taken this outside of the loop in preparation for threading the loading of channels.
  • Transparent background after opening / loading as file layer.
Test Plan

For testing the crash:

  • Create a new image with 8-bit integer color depth.
  • (optional) Paint a beautiful tree
  • Add some layers
  • Save as .exr and check 'flatten'.
  • Open .exr to see a single layer with a beautiful tree

Testing display w/ data window requires a cropped .exr file. (Blender may be able to output this)

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jhoolmans requested review of this revision.May 18 2018, 9:29 PM
jhoolmans created this revision.
rempt accepted this revision.May 19 2018, 7:03 AM
rempt added a subscriber: rempt.

Nice! Just two remarks, none of them essential, but may be helpful.

plugins/impex/exr/exr_converter.cc
322

We usually don't use this-> if it isn't necessary.

385

If you port this to KisSequentialIterator, it'll be even faster.

448

Same here. Dmitry says, hlineiterator is about twice slower :-)

This revision is now accepted and ready to land.May 19 2018, 7:03 AM
jhoolmans added a comment.May 19 2018, 9:24 AM

Ah that sounds great. Maybe I can do the same with saving also.

The most noticeable slowdown is loading the view with larger 32-bit images.
Maybe we can look at that after these optimizations.

jhoolmans marked 2 inline comments as done.May 19 2018, 9:10 PM
jhoolmans added inline comments.
plugins/impex/exr/exr_converter.cc
322

Removing this-> actually makes my code crash. I believe it's necessary since it's a member of Private? Or maybe I'm missing something.

jhoolmans updated this revision to Diff 34492.May 19 2018, 9:42 PM
rempt added a comment.May 20 2018, 7:48 AM

That's very strange -- it really shouldn't be necessary, and there's no crash on my systems, but oh well, let's keep it then.

jhoolmans updated this revision to Diff 34507.May 20 2018, 10:13 AM
jhoolmans edited the summary of this revision. (Show Details)

You're right, didn't crash this time. Must've done something wrong.

Also updated the background color after loading an exr to be fully transparent.

If there are no further remarks I will push it to master. Thanks!

rempt added a comment.May 20 2018, 4:02 PM

It's fine by me!

Closed by commit R37:c94a09030c86: OpenEXR loading performance optimizations. (authored by jhoolmans). · Explain WhyMay 20 2018, 7:23 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 34548

View Options

plugins/impex/exr/exr_converter.h

Loading...
View Options

plugins/impex/exr/exr_converter.cc

Loading...
View Options

plugins/impex/exr/exr_export.cc

Loading...