Skip to content

Fix copy/paste and module iop order setting. - #22483

Open
TurboGit wants to merge 3 commits into
masterfrom
po/fix-copy-paste
Open

TurboGit wants to merge 3 commits into
masterfrom
po/fix-copy-paste

Conversation

@TurboGit

@TurboGit TurboGit commented Oct 2, 2026

Copy link
Copy Markdown
Member

Summary

The previous selection of iop-order in selective copy is not used as default when doing a full copy/paste.

Also in non selective copy we now copy the module iop order.

Referenced issue

Related to #6528.

Closes #22426.

Checklist

  • I have read CONTRIBUTING.md and the coding style.
  • I have not merged master into the topic branch.
  • The pull request is one logical change, and every commit compiles on its own.
  • I ran the relevant tests: unit tests, src/tests/integration/ where the pixelpipe is touched, or darktable-cli as a headless smoke test.
  • New user-visible strings use _(), new preferences are registered in data/darktableconfig.xml.in.
  • A RELEASE_NOTES.md entry was added.

Test instructions

Manual full and selective copy with and without the iop-order.

AI assistance

For GUI code.

@TurboGit TurboGit added this to the 5.8 milestone Oct 2, 2026
@TurboGit TurboGit added priority: medium core features are degraded in a way that is still mostly usable, software stutters feature: redesign current features to rewrite scope: codebase making darktable source code easier to manage default-behavior-change ai:generated labels Oct 2, 2026
@anoderay anoderay added the documentation: pending a documentation work is required label Oct 2, 2026
@anoderay

anoderay commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Compilation of this PR failed for me:

src/develop/masks/masks.c:1276:5: error: 'before.group_selected' may be
used uninitialized [-Werror=maybe-uninitialized]
In function '_gui_hover_state_equal',
    inlined from 'dt_masks_events_mouse_moved' at masks.c:1359:14

Must be a problem with master though, cannot compile master with the same error.

@TurboGit

TurboGit commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

It compiles for me and master also. That's strange because all PR merged on master have compiled ok on the CI.

@anoderay anoderay mentioned this pull request Oct 2, 2026
1 task done
@da-phil

da-phil commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

It compiles for me and master also. That's strange because all PR merged on master have compiled ok on the CI.

In this case it is actually a compiler false-positive finding by an old GCC12 toolchain, no other more modern toolchain seems affected by this. hence our GCC16 based CI check didn't catch it, and I assume you're also building code with a more up-to-date toolchain on your PC ;)
More info here: #22487

Comment thread src/gui/hist_dialog.c Outdated
The previous selection of iop-order in selective copy is not
used as default when doing a full copy/paste.

Also in non selective copy we now copy the module iop order.

Related to #6528.

Closes #22426.
@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

Works for me.

@da-phil

da-phil commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

I asked Claude for a review and confirmed the following finding:

Cancel or Escape doesn't undo a change to the check box (src/gui/hist_dialog.c:49, :355)

  • The copy and paste dialogs change the global copy buffer before the user confirms, and cancel or escape doesn't restore it. New in this PR: the module order check box writes copy_iop_order as soon as it is clicked. Older, but made worse here: dt_history_copy_parts() repoints the buffer and sets full_copy = TRUE before the dialog runs. So a cancelled selective copy throws away the previous copy, and the next paste copies everything from the new image, including the unsafe-to-copy modules and now the module order. full_copy = TRUE only takes effect when nothing is selected, so the comment above it doesn't describe what it does. Running the dialog on a local copy of the item and writing it back only on OK/APPLY would fix all of this.
  • Example: you ctrl+c, open selective paste, untick "module order", then cancel or press escape. Every later plain ctrl+v from that copy silently leaves out the module order.
  • Fix: read the check box in _gui_hist_copy_response for OK/APPLY only, for example by storing the widget with g_object_set_data on the dialog.

And here is more of a design question:

Plain paste now always replaces the target's order. This is intended, but has side effects worth discussing (history.c:945-960)

  • Plain ctrl+v is append (control_jobs.c:2678), and the source's order list is now written to every target.
  • Copying from an unaltered image used to change nothing. Now it resets each target's order, including hand-arranged ones.
  • Copying from a JPEG (which uses the v5.0 JPEG order) onto raws gives the raws the JPEG order.
  • Worth asking the developers: should the order only travel when the source has history, or when its order is DT_IOP_ORDER_CUSTOM?

@anoderay

anoderay commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Did some testing and to me it works as I'd expect :-). This is a useful change IMHO!

Only thing is: The module border checkbox looks a bit lost. It could use a nudge to the right and is missing a "verb" as it is outside of the table now. So for both the selective copy and paste dialog I suggest adding "include" after the checkbox.

@TurboGit

TurboGit commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Plain paste now always replaces the target's order. This is intended, but has side effects worth discussing

Yes, that's a side effect and also if you copy/paste to/from different RAW file, you can get a wrong WB setting. That's probably one reason we avoided full copy/paste to also get the iop-order to mitigate the issue. As I see things now, the full copy/paste is designed for similar picture same RAW, same WB, same lens... and if we thinks about it it is ok. It is a way to synchronize a dev to multiple "similar" images. For all other cases one need to use selective copy/paste.

@TurboGit

TurboGit commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Cancel or Escape doesn't undo a change to the check box

Should be fixed now.

@wpferguson

Copy link
Copy Markdown
Member

Did a quick test. Applied a style, added a couple of modules, then finished the edit. Did ctrl-c to copy and ctrl-v to paste. The edit looked the same, but an instance of local contrast was added to the history stack but left turned off. The copied edit didn't use the local contrast.

I wrote this last night when I was tired and forgot to post it. I'll do some more checking now that I'm somewhat awake again.

@TurboGit

TurboGit commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

The edit looked the same, but an instance of local contrast was added to the history stack but left turned off. The copied edit didn't use the local contrast.

That's weird, I don't even know how this is possible. Can you reproduce consistently?

@wpferguson

wpferguson commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Consistently...

Here's the presets and style I'm using

presets_and_style.zip

To recreate my steps

  • unzip the zip files and import the presets and style
  • open an image in darkroom
  • set the modulegroup to portrait preset
  • apply the style
  • turn on
    • both instances of diffuse and sharpen
    • colorbalancergb
    • add contrast and texture. Increase fine detail a little and decrease coarse and medium detail
    • add an instance of contrast eqalizer. Lower luma on on the left side slightly.
  • return to lighttable
  • copy and paste using the history stack module or ctrl-c ctrl-v

Check the history stack and there is a local contrast entry at the top of the history stack that is not turned on.


In the meantime, I'll try not using presets and styles to see if it's just something funny with those.


Edit: If I don't use the style or presets then the copy/paste appears to work fine.

@wpferguson

Copy link
Copy Markdown
Member

The style did have a turned off instance of local constrast. Here's what the source of the copy looked like and what the pasted stack looked like

Source

source

Pasted

pasted

@wpferguson

Copy link
Copy Markdown
Member

I got an error when reimporting previous edits (#22504). I wonder if the style I was using, which was probably created in dt 5.0, somehow stored IOP order that has since been updated and that's what is causing the distorted paste?

@TurboGit

TurboGit commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

The style did have a turned off instance of local constrast. Here's what the source of the copy looked like and what the pasted stack looked like

I don't think that it is an issue. On the source local contrast is OFF on history step 14. On target it is OFF on history step 25. And you know that the order in the history is not the order of the pipe anyway. On my side the target looks fine with the custom module ordre and the modules at the same place in the pipe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated default-behavior-change documentation: pending a documentation work is required feature: redesign current features to rewrite priority: medium core features are degraded in a way that is still mostly usable, software stutters scope: codebase making darktable source code easier to manage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

History stack copy-paste does not preserve module order

5 participants