Skip to content

GSOC 2023: Initial non-destructive editing implementation

Alx Sa requested to merge alxsa-initial-nde-checkin into master

Initial implementation of non-destructive editing. I believe it's ready to be reviewed again when there's time, to see what needs to be fixed (or if I forgot to implement anything).

Original Version

This is an initial "check-in" MR to ask for some feedback on my current direction. The code is ugly but functional (if a bit delicate).

You should be able to do the following:

  • Apply filters destructively or non-destructively (via the "As Layer Effects" button tacked on to the filter tools dialogue)
  • On the "Edit Layer Attributes" dialog, you can clear out all NDE filters or merge them down destructively.

Video showing current functionality

GIMP_WIP_-_Two_NDE_GUI_Options

UPDATE 1: Individual filters can now be removed by selecting them in the "Edit Layer Attributes" dialogue and choosing "Remove Filter".

UPDATE 2: Layer tree UI (basic) now in place, and filters are non-destructive by default.

General Outline

  • A new "temp_filter_stack" GimpContainer is added in-between the layer data and the existing filter_stack in gimpdrawable-private.h
  • Filters are initially added to to this temp_filter_stack when the filter dialogue opens
    • If the user chooses "Ok", the filter is applied destructively as before.
    • If the user chooses "As Layer Effect", the filter is removed from "temp_filter_stack" and added to "filter_stack". gimp_drawable_merge_filter () is not called which allows the filter to live on.
  • "Merge Filters" works by iterating through the filters in the filter_stack GimpContainer and calling gimp_drawable_merge_filter () on each individually
  • "Clear Filters" works by clearing out the filter_stack GimpContainer
    • Note: This doesn't seem to trigger the canvas to refresh. I have a temporary hack in place to force a change via opacity, but I'd need to tie this in GimpDrawable somehow.

Some Specific Questions

  • Is having an additional "temporary" filter stack for destructive filters a good approach?

    • In general, how should the preview for destructive/non-destructive edits work? Currently we don't know which mode the user will choose when they apply a filter, so the preview can be off (I assume that destructive filters should be applied on the bottom right before the layer data, while non-destructive filters by default go on the top of the stack)
  • Does it make sense to have a new tool enum, GIMP_TOOL_ACTION_NDE_COMMIT? I added it so I can have a different commit option in the dialogue, but I assume that in the future other tools could be applied non-destructively (e.g. scale/rotate). If this is considered a good idea, I'll update the other switch cases to get rid of the warnings :)

  • Currently I created two copies of functions for applying/committing/removing filters - one for destructive edits, one of non-destructive. It was mostly done so this would be a self-contained prototype. Is this preferred in some cases, or should I be adding flags to the existing functions to determine whether to apply effects destructively/non-destructively?

  • The next feature to implement (after cleaning up what's already here) is re-arranging/removing/changing the settings of individual features. Should I add functionality to GimpContainer (data structure) and GimpContainerTreeView (GUI) to handle retrieving, selecting, and activating individual filters, or should I make subclasses (maybe GimpFilterContainer and GimpFilterTreeView)? Or are those functions already there and I'm just missing them? :)

  • General comments on the existing structure and what I should improve or change?

Edited by Alx Sa

Merge request reports