Fix 404615: Dimensions of Images output by "Export Layers" vary wildly depending on the layers alpha
ClosedPublic

Authored by vanyossi on Apr 15 2019, 1:37 PM.

Details

Summary

The save function in Node.cpp uses current layer bounds to create new image to save, this is what causes the layer to only export cropped size.

Added a flag to allow selecting crop to image or crop to layer boundaries, as each is usefull in differente workflows. (I normally use crop to image bounds)

Test Plan

Exporting layers work as expected,

  • one image size for all output layers

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.
vanyossi created this revision.Apr 15 2019, 1:37 PM
Restricted Application added a reviewer: Krita. · View Herald TranscriptApr 15 2019, 1:37 PM
vanyossi requested review of this revision.Apr 15 2019, 1:37 PM
vanyossi edited the test plan for this revision. (Show Details)Apr 15 2019, 1:40 PM

This is a bit dangerous indeed, as it is currently not possible to 'autocrop' layers, meaning that plugins which means plugins that rely on autocrop behaviour will break without a way to undo it.

@razcore.art how do you feel about thsi?

dkazakov added inline comments.
libs/libkis/Node.cpp
548

I think a safer approach would be:

QRect bounds = d->node->exactBounds() | d->image->bounds();

With dmitry's proposal we can have grow and fit to image export layers behaviour.

Trimming image before export keeps all layers at the same size, however it is not possible to get smaller layers, or "trim" the alpha excess froma layer automagically.

libs/libkis/Node.cpp
548

Tested change and as suspected,

  • bigger layer are exported bigger than canvas.
  • Smaller layers remain canvas size.

The First one can be fixed by triming the image before export.
The second one, in case you need cropping non existent data, is not possible.

vanyossi updated this revision to Diff 56342.Apr 16 2019, 2:05 AM

Dmitry solution does not allow to get all layers in the same size. Nor is it possible to have smaller cropped layers.

Updated save funtion to receive a "bool" to select if you want image boundary size or layer boundary size.
Default value is "cropped to layer boundaries" (as before)

in python export_layers.py added a checkbox to select the type of export needed.

rempt added a subscriber: rempt.Apr 16 2019, 7:51 AM
rempt added inline comments.
libs/libkis/Node.h
487

a bool parameter shouldn't have an int as default value, but false (or true)

vanyossi marked an inline comment as done.Apr 16 2019, 2:01 PM
vanyossi added inline comments.
libs/libkis/Node.h
487

Oh my, I was in C mode aparently D: Fixing…

vanyossi updated this revision to Diff 56490.Apr 18 2019, 5:37 AM
vanyossi marked an inline comment as done.
vanyossi edited the summary of this revision. (Show Details)
vanyossi edited the test plan for this revision. (Show Details)

Updated: used booleans for booleans and not ints

dkazakov added inline comments.Apr 20 2019, 9:27 AM
libs/libkis/Node.cpp
548

I'm still not convinced that this boolean switch a good idea from API point of view. There are three possible usecases of this saving API. And the current API doesn't cover them all:

  1. The caller wants to export the layer's contents exactly how he user sees it (then the the caller uses cropToImageBounds switch)
  1. The caller wants to export the layer's contents and preserve as much pixel data as possible. Transparent data is not considered as "data". In such a case we need to export d->node->exactBounds() area.
  1. The caller wants to export the layer's contents and preserve as much pixel data as possible. Transparent data is considered as "data". In such a case we need to export d->node->exactBounds() | d->image->bounds() area.

With the current switch-like API, the third option is not possible (and in Krita we use such rect quite a log).

What if the API will be like:

/**
 * If \p exportRect is empty, then the method saves exactBounds() of the node. If you'd like to save the image-
 * aligned area of the node, just pass image->bounds() there.
 */
void save(const QString &filename, 
          double xRes, double yRes, 
          const InfoObject & exportConfiguration, 
          const QRect &exportRect = QRect());
This revision was not accepted when it landed; it landed in state Needs Review.Apr 24 2019, 6:14 AM
This revision was automatically updated to reflect the committed changes.