RFC: expose final scale iop to gui #13635

MStraeten · 2023-02-13T22:30:15Z

taking over code from https://github.com/aurelienpierreeng/ansel/blob/24b11d76c8ad097b1b1657697ac012fe4ded515b/src/iop/finalscale.c to enable custom position of the final scaling iop in case there's a demand to apply scaling earlier in the export pipe to e.g. allow sharpening after rescaling.

line 164 contains a link to ansel documentation of usecases - maybe to be replaced by something else ...

since that allows an uswer to shoot themselves into the foot - feel free to discuss

taking over code from https://github.com/aurelienpierreeng/ansel/blob/24b11d76c8ad097b1b1657697ac012fe4ded515b/src/iop/finalscale.c to enable custom position of the final scaling iop in case there's a demand to apply scaling earlier in the export pipe to e.g. allow sharpening after rescaling. line 164 contains a link to ansel documentation of usecases - maybe to be replaced by something else ... since that allows an uswer to shoot themselves into the foot - feel free to discuss

TurboGit · 2023-02-14T12:59:09Z

As I said (and others) here #13335, this doesn't seems to solve the issue. So it has been discussed but up to now there is no proof that this helps in any way. Do you have some positive experiences to share?

jenshannoschwalm · 2023-02-14T15:42:41Z

It helps insofar, as you can do things like sharpening after scaling .

Also, this not only about downscaling.

TurboGit · 2023-02-14T16:16:35Z

It helps insofar, as you can do things like sharpening after scaling .

Right, this is indeed important.

jenshannoschwalm · 2023-02-14T16:52:04Z

It am yet not sure if a "ask the user to change pipeline order" is the right way.

Also upscaling should be more restricted. Modules like d&s might go wild easily.

MStraeten · 2023-02-14T17:18:18Z

main noticeable difference is sharpening - that color stuff seems to be an edge case.
Rescaling first then do D&S stuff might be also useful to mitigate noise - indeed - since it's not wysiwyg users can easily shoot themselves into the foot.

maybe a more appropriate solution for applying a sharpening style after finalscale might be found just for export

TurboGit · 2023-02-14T17:39:25Z

maybe a more appropriate solution for applying a sharpening style after finalscale might be found just for export

Probably so yes. We can certainly have an option in export to add a sharpen instance in the pixelpipe (as we do for style). The problem is how to control the sharpen strength/level directly in the export module? We can have a combo with none/soft/medium/strong option for the sharpening, would that be enough?

Lot of questions...

spaceChRiS · 2023-02-14T18:56:58Z

I wonder if it would be a better solution to introduce a regular “scale” module that allows to scale within the regular pixel pipe, which could be used together with one of the sharpening modules at the end of the pipe to post-scale sharpen. This would allow to preview the result, and could be applied as style at export by those who do regular post-scale sharpening.

This would furthermore fix a second issue that I regularly have. I often have images of people that are important but not 100 % sharp. Therefore I want to limit the output resolution to an amount that hides the imperfection. This could be achieved with the same scale module within the regular pipe, but this time not as preset but simply as part of the pipe.

What do you think?

Edit: style, not preset in the first paragraph.

SoupyGit · 2023-02-14T23:11:28Z

We can certainly have an option in export to add a sharpen instance in the pixelpipe (as we do for style). The problem is how to control the sharpen strength/level directly in the export module? We can have a combo with none/soft/medium/strong option for the sharpening, would that be enough?

There are perhaps too many different sharpening methods and ways to fine tune them for a simple solution like this to please all. In the case of downscale, scene referred, and good gpu, d&s will most likely be the preferred option, in which case selecting a preset for that module from export would be a solution. But what about upscale, display referred, or no/poor gpu?

Seems the only way to cover all use cases is to expose final scale to user in pipeline. I agree with @MStraeten the most efficient workflow for this will be to apply a style on export.

What is the drawback of exposing final scale to user in pipe? I don't really see one.

jenshannoschwalm · 2023-02-15T04:27:53Z

What is the drawback of exposing final scale to user in pipe? I don't really see one.

I don't see either.

Just the description "This module is used to downscale images at export time" is simply wrong, I haven't tested that but i am not even sure that downscaling uses this at all with current pipeline flow.

TurboGit · 2023-02-15T09:44:21Z

What is the drawback of exposing final scale to user in pipe? I don't really see one.

One more module visible :) I personally don't have objection.

MStraeten · 2023-02-15T13:07:13Z

i found the module is just visible if you used it in a custom pipe order - by default it's not displayed unless you enable display of all modules ... so maybe not confusing most users

for a general use (without high quality resampling) there's no need to bother with final resample position since resampling is already done early in the pipe to speed up things - no difference visible ;)

the effect of sharpening after final resample is visible if you're using high quality resampling, so quite useful for that specific usecase:

jenshannoschwalm · 2023-02-17T05:54:57Z

for a general use (without high quality resampling) there's no need to bother with final resample position since resampling is already done early in the pipe to speed up things - no difference visible ;)

That's probably the point for the "color issue".

jenshannoschwalm · 2023-02-17T05:57:02Z

src/iop/finalscale.c

+{
+  IOP_GUI_ALLOC(finalscale);
+  self->widget = gtk_label_new(NULL);
+  gtk_label_set_markup(GTK_LABEL(self->widget),_("This module is used to downscale images at export time. "


I think this is plain wrong.

We should in any case remove all capital letters to keep standard dt style.

@jenshannoschwalm : Plain wrong? You mean that export time is confusing? Because to me this module does clip/zoom to final size looking at the code. This module is indeed used for all pixelpipe.

capitals removed

-d pipe indicates the existence of finalscale just on export if high quality resampling is active:

299,0892 [modify roi OUT] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 299,0892 [modify roi IN] [export] finalscale ( 0/ 0) 5202x3460 scale=1,0000 --> ( 0/ 0) 648x 431 scale=0,1246 299,0892 [modify roi IN] [export] highlights ( 0/ 0) 5202x3464 scale=1,0000 --> ( 0/ 0) 5202x3460 scale=1,0000 299,0892 [modify roi IN] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 299,0927 [pixelpipe_process_CL] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 ... 299,3029 [pixelpipe_process_CL] [export] colorout ( 0/ 0) 5202x3460 scale=1,0000 --> ( 0/ 0) 5202x3460 scale=1,0000 299,3447 [pixelpipe_process_CL] [export] finalscale ( 0/ 0) 5202x3460 scale=1,0000 --> ( 0/ 0) 648x 431 scale=0,1246 299,3448 [clip_and_zoom_roi CL] [export] finalscale ( 0/ 0) 5202x3460 scale=1,0000 --> ( 0/ 0) 648x 431 scale=0,1246 device=0 299,3584 [cache report] [export] 2 lines (important=0, used=2). Used 575MB, limit=0MB. Hitrate=0,00

export no high quality resampling:

292,7390 [modify roi OUT] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 292,7390 [modify roi IN] [export] demosaic ( 0/ 0) 5202x3459 scale=1,0000 --> ( 0/ 0) 648x 431 scale=0,1246 292,7391 [modify roi IN] [export] highlights ( 0/ 0) 5202x3464 scale=1,0000 --> ( 0/ 0) 5202x3459 scale=1,0000 292,7391 [modify roi IN] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 292,7434 [pixelpipe_process_CL] [export] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 ... 292,8721 [pixelpipe_process_CL] [export] colorout ( 0/ 0) 648x 431 scale=0,1246 --> ( 0/ 0) 648x 431 scale=0,1246 292,8742 [pixelpipe_process_on_CPU] [export] gamma ( 0/ 0) 648x 431 scale=0,1246 --> ( 0/ 0) 648x 431 scale=0,1246 292,8745 [cache report] [export] 2 lines (important=1, used=2). Used 575MB, limit=0MB. Hitrate=0,00

full pipe:

274,7225 [modify roi OUT] [full] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 274,7225 [pixelpipe_cache_checkmem] [full] 64 lines (important=0, used=0). Cache: freed=0MB (bad=0 low=0 high=0). Now using 0MB, limit=512MB 274,7225 [modify roi IN] [full] demosaic ( 0/ 0) 5202x3462 scale=1,0000 --> ( 0/ 0) 2096x1395 scale=0,4029 274,7225 [modify roi IN] [full] highlights ( 0/ 0) 5202x3464 scale=1,0000 --> ( 0/ 0) 5202x3462 scale=1,0000 274,7226 [modify roi IN] [full] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 274,7262 [pixelpipe_process_CL] [full] rawprepare ( 0/ 0) 5360x3516 scale=1,0000 --> ( 0/ 0) 5202x3464 scale=1,0000 ... 274,8739 [pixelpipe_process_CL] [full] colorout ( 0/ 0) 2096x1395 scale=0,4029 --> ( 0/ 0) 2096x1395 scale=0,4029 274,8826 [pixelpipe_process_on_CPU] [full] gamma ( 0/ 0) 2096x1395 scale=0,4029 --> ( 0/ 0) 2096x1395 scale=0,4029 274,8869 [cache report] [full] 64 lines (important=2, used=10). Used 518MB, limit=512MB. Hitrate=0,00

so the phrase "used to downscale images at export time" might not be that precise but not wrong.
maybe better " ...used to downscale images with high quality resampling"

Indeed, you're right and this is new to me. The module finalscale is only used for high quality export.

Yes, but also for upscaling

src/iop/finalscale.c

and also removed link to ansel documentation

MStraeten · 2023-02-17T18:02:27Z

question: how to get rid of the empty widget usually populated with const char **description(struct dt_iop_module_t *self) {...} stuff?

TurboGit · 2023-02-17T18:09:26Z

The description() routine is used to populate the tooltip when flying over the module name. So you are probably looking for something else.

MStraeten · 2023-02-17T18:52:27Z

so how to get rid of the tooltip? it seems to be a tooltip activated when hovering over the name of the module.

changed notice

jenshannoschwalm · 2023-02-18T08:46:39Z

src/iop/finalscale.c

@@ -159,7 +159,7 @@ void gui_init(dt_iop_module_t *self)
 {
  IOP_GUI_ALLOC(finalscale);
  self->widget = gtk_label_new(NULL);
-  gtk_label_set_markup(GTK_LABEL(self->widget),_("this module is used to downscale images at export time. "
+  gtk_label_set_markup(GTK_LABEL(self->widget),_("this module is used to downscale images with high quality resampling mode at export time. "


It's about upscaling too :-)

TurboGit · 2023-02-18T08:47:00Z

Now that I see that this module is only used at export time I would certainly prefer having some new option in export module. What I don't like here is that we introduce a new module that is visible in the pixelpipe and for the first time this module is of no used in darkroom.

Of course I'm ready to be convinced that this is the right solution but I'm a bit uneasy to introduce this at this point.

jenshannoschwalm · 2023-02-18T08:54:50Z

Now that I see that this module is only used at export time I would certainly prefer having some new option in export module. What I don't like here is that we introduce a new module that is visible in the pixelpipe and for the first time this module is of no used in darkroom.

Yes and also the "module on/off" in export depends on export parameters like hq-scaling.

While in darkroom the scaling always happens in demosaicing for raws.

This difference - darkroom vs export scaling - means that all modules after the demosaicer get different data to work on ...

TurboGit · 2023-02-18T09:19:10Z

So a better path is probably:

Introduce a new hidden module named finalsharpen
Make this module disabled by default
This new module has a single parameter strength to allow for setting the amount of sharpening (0 = None)
A new slider "final sharpening" is introduced into the export module (0 by default) up to (100%)
When this slider is > 0 during export the finalsharpen module is enabled
The actual strength is passed to it using config key

The finalsharpen being a new module the sharpening algorithm may be whatever is best for the use case.

I think this is more user friendly and does not add another level of complexity to the user by requiring to move modules around.

spaceChRiS · 2023-02-18T10:28:42Z

Introduce a new hidden module named finalsharpen

May I ask what would be the drawback (besides that it needs to be implemented) of having such a module visible in the pipe, together with the final scaling operation just before this step (maybe in the same module), where scale parameters could be entered (either as wxh pixels or as factor)? This would at least allow to preview the outcome of post-scaling sharpening, even if the scaling is then switched off and an export without high-q resampling and therefore early scaling would be used in combination with export sharpening.

The scaling could be disabled by default as well as the sharpening, with a checkbox that says “sharpening controlled by export module”, and only if this checkbox is unchecked, the sharpening settings given in the module would apply. The scaling would also have such a checkbox, which in unchecked state would limit the maximum export size (smaller exports would be allowed) by the given resolution or scaling factor.

parafin · 2023-02-18T10:43:37Z

So a better path is probably:

Introduce a new hidden module named finalsharpen

Make this module disabled by default

This new module has a single parameter strength to allow for setting the amount of sharpening (0 = None)

A new slider "final sharpening" is introduced into the export module (0 by default) up to (100%)

When this slider is > 0 during export the finalsharpen module is enabled

The actual strength is passed to it using config key

The finalsharpen being a new module the sharpening algorithm may be whatever is best for the use case.

I think this is more user friendly and does not add another level of complexity to the user by requiring to move modules around.

This solution ignores the fact that hq export does scaling at the wrong place in the pipe (after gamma and not before). Perhaps finalscale could undo and redo gamma in itself?..

MStraeten · 2023-02-18T12:58:47Z

So two usecases that might be better separated:

allowing to sharpen after rescaling in high quality resampling export mode —> better handled and more ‚intuitive‘ (i expect setting that when I set the final size and not in an export agnostic edit pipe)
do the final scaling at a proper place before leaving the unbounded scene referred section of the pipe - but that can be done without exposing the module in the gui

having the module exposed or not doesn’t matter that much since you won’t see any differences in darkroom…

TurboGit · 2023-02-18T13:06:41Z

May I ask what would be the drawback (besides that it needs to be implemented) of having such a module visible in the pipe

A visible module that does nothing in darkroom and only enabled at export time is kind of messy to me.

TurboGit · 2023-02-18T13:08:30Z

This solution ignores the fact that hq export does scaling at the wrong place in the pipe (after gamma and not before). Perhaps finalscale could undo and redo gamma in itself?..

That's an orthogonal issue or do I miss the point?

One of the goal discussed here was to be able to sharpen after scaling hence my proposal.

spaceChRiS · 2023-02-18T14:21:20Z

A visible module that does nothing in darkroom and only enabled at export time is kind of messy to me.

I was not describing a module that does nothing. I was describing a module that works within darkroom pixel pipe and scales the image in darkroom as an alternative to the suggested solution to expose the current final scale IOP which indeed would do nothing in darkroom. Effectively this may swap final scaling and gamma, which may not be too much of an issue unless I miss something. Introduced as an additional module (additional to the final scale IOP) would even not break current behavior, but maybe it is also worth to replace final scale IOP by this new approach (not sure how much of an impact a scale/gamma switch would have on old edits).

Additional features that would be enabled by this change were described by my post above.

If my suggestions are not appropriate or do not belong here, please tell me, this is marked RFC therefore I thought it is OK to post my suggestion of an alternative solution to (part of) the problem which is sharpen after scale.

pcanning · 2023-02-18T22:19:21Z

Just FYI, one very popular commercial photo editing application that has sharpening controls in the export dialog includes 3 controls: a checkbox to enable/disable output sharpening; a drop-down menu to specific sharpening for screen, matte paper, or glossy paper; and an amount drop-down menu to specify the amount of sharpening as low, standard, or high.

SoupyGit · 2023-02-18T23:10:05Z

Exposing final scale in darkroom doesn't do nothing. Yes, it doesnt have parameters of its own, but it allows you to move it before the tone curve (eg. Filmic), thus allowing to downscale on linear data. The benefit of this is that a sharpening module (d&s, sharpen, contrast eq - users choice) can be placed in between final scale and tone curve. Placing sharpening after final scale is the equivalent of post-resize sharpening, and doing this before tone curve allows the sharpening to occur on linear data, which is less likely to produce artifacts (and if we use d&s, is specifically designed for linear data).

If you dont want to expose this to user in darkroom but do it in export, then two things would happen:

A reorder of final scale in pixelpipe when hq resampling is on & output dimensions are smaller than input (ie, downsize not upsize)
Post-resize sharpening option.

What's the pros and cons of doing it in export?

Pros: simpler interface, less tedious for user to change module order than in darkroom, user doesn't need different darkroom settings for different export sizes. Cons: User cannot confirm the accuracy of pixelpipe reorder. Shouldn't be a problem in most cases, but in the past sometimes when copying/pasting history or doing custom orders with multiple module instances there have been bugs, and modules end up in the wrong spot. There would be no way for user to check this.
Pros: simpler interface. Cons: no way for user to choose their own sharpening algo. For instance, user with slow/no gpu might want sharpen or contrast eq for speed, while user with good gpu might want d&s. Even with d&s, different users might prefer different presets. User who upscale might prefer different algo to user who downscales. Users who don't reorder pipe and thus downsize on non linear data might prefer different algo to those who do. As we can see, this means the position of post resize sharpening module is not constant in pipe order either, depending on option chosen.

The only way I can possibly see 2 working in export module is you have two options to select:
A) module for sharpening
B) allow to select a preset in that module (instead of a strength slider)

Could a new module final sharpen achieve all of the above?

I'm not against that idea, just to make sure it covers all the various use cases.

The other question to ask is whether moving final scale helps solve any other problem, such as the 'color' one that was initially raised. No one seems to have an answer to that. If it did then the importance of moving final scale would not be linked to post resize sharpening only.

jenshannoschwalm · 2023-02-19T08:17:58Z

The other question to ask is whether moving final scale helps solve any other problem, such as the 'color' one that was initially raised. No one seems to have an answer to that.

It doesn't fully. That problem is currently already defined by downscaling in the demosicer stage from my analysis. The available test image (as non-raw) doesn't make this obvious.

That issue is certainly 'high priority'

TurboGit · 2023-02-19T17:00:45Z

That issue is certainly 'high priority'

@jenshannoschwalm : If you plan working on this, please take the lead I'm really not sure to understand everything involved here. We can certainly team together to work on this and if my proposal above is part of the solution I can handle this. Please let me know, we may want to open a new issue to discuss all the implications and the way forward. Do you want me to open such issue?

jenshannoschwalm · 2023-02-19T17:33:32Z

@TurboGit yes i am planning to do so pretty soon. Just two things in my pipeline a) get something better for OpenCl intels and b) embedded lens correction also for OpenCl. Maybe a month or so? team together sounds good, i will certainly ask what @garagecoder and Iain think about this, they don't know the dt codebase but always have good ideas about color maths.

jenshannoschwalm · 2023-02-19T17:34:38Z

Any yes, an issue before designing is good.

TurboGit · 2023-02-19T17:43:38Z

Any yes, an issue before designing is good.

Done here #13682.

MStraeten · 2023-02-19T18:15:06Z

so this can be closed, it doesn't make sense to merge a patchy manual solution for something that will be solved with a proper design ;)

This fix color & luminosity shift when HQ mode is used. Also use a new DT_DEFAULT_IOP_ORDER define and use it consistently where needed. Less maintenance needed when a new order is introduced. Fixes #17758, #13335, #13635 and #13682.

Merge branch 'darktable-org:master' into MStraeten-finalscale

46a201c

jenshannoschwalm reviewed Feb 17, 2023

View reviewed changes

src/iop/finalscale.c Outdated Show resolved Hide resolved

lower case description

f7293fe

and also removed link to ansel documentation

indicate use for high quality resampling

a63a5b6

changed notice

jenshannoschwalm reviewed Feb 18, 2023

View reviewed changes

MStraeten closed this Feb 19, 2023

TurboGit mentioned this pull request Feb 20, 2023

rework the final-scale and make it possible to have a sharpen after downscaling #13682

Open

jenshannoschwalm mentioned this pull request Nov 5, 2024

Image before export is too red, but after it is too green. #17758

Closed

RFC: expose final scale iop to gui #13635

RFC: expose final scale iop to gui #13635

Conversation

MStraeten commented Feb 13, 2023

TurboGit commented Feb 14, 2023

jenshannoschwalm commented Feb 14, 2023

TurboGit commented Feb 14, 2023

jenshannoschwalm commented Feb 14, 2023

MStraeten commented Feb 14, 2023

TurboGit commented Feb 14, 2023

spaceChRiS commented Feb 14, 2023 • edited Loading

SoupyGit commented Feb 14, 2023

jenshannoschwalm commented Feb 15, 2023

TurboGit commented Feb 15, 2023

MStraeten commented Feb 15, 2023

jenshannoschwalm commented Feb 17, 2023

jenshannoschwalm Feb 17, 2023

Choose a reason for hiding this comment

TurboGit Feb 17, 2023

Choose a reason for hiding this comment

TurboGit Feb 17, 2023

Choose a reason for hiding this comment

MStraeten Feb 17, 2023 • edited Loading

Choose a reason for hiding this comment

TurboGit Feb 18, 2023

Choose a reason for hiding this comment

jenshannoschwalm Feb 18, 2023

Choose a reason for hiding this comment

MStraeten commented Feb 17, 2023

TurboGit commented Feb 17, 2023

MStraeten commented Feb 17, 2023

jenshannoschwalm Feb 18, 2023

Choose a reason for hiding this comment

TurboGit commented Feb 18, 2023

jenshannoschwalm commented Feb 18, 2023

TurboGit commented Feb 18, 2023

spaceChRiS commented Feb 18, 2023

parafin commented Feb 18, 2023

MStraeten commented Feb 18, 2023

TurboGit commented Feb 18, 2023

TurboGit commented Feb 18, 2023

spaceChRiS commented Feb 18, 2023

pcanning commented Feb 18, 2023

SoupyGit commented Feb 18, 2023 • edited Loading

jenshannoschwalm commented Feb 19, 2023

TurboGit commented Feb 19, 2023

jenshannoschwalm commented Feb 19, 2023

jenshannoschwalm commented Feb 19, 2023

TurboGit commented Feb 19, 2023 • edited Loading

MStraeten commented Feb 19, 2023

spaceChRiS commented Feb 14, 2023 •

edited

Loading

MStraeten Feb 17, 2023 •

edited

Loading

SoupyGit commented Feb 18, 2023 •

edited

Loading

TurboGit commented Feb 19, 2023 •

edited

Loading