Skip to content

Removed dependencies in certain document-related classes #2073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lehonti wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Removed dependencies in certain document-related classes #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c135bfb
Removed `ToolManager` dependency from `DocumentWorkspace`
Lehonti Mar 24, 2026
ff6d6be
Removed dependency of `EditActions` in `DocumentHistory`
Lehonti Mar 24, 2026
0f3ee48
Merge branch 'master' into improvement1
Lehonti Mar 26, 2026
53b43bc
Merge branch 'PintaProject:master' into improvement1
Lehonti Mar 28, 2026
5d43877
Merge branch 'PintaProject:master' into improvement1
Lehonti Mar 31, 2026
33331b7
Merge branch 'PintaProject:master' into improvement1
Lehonti Apr 2, 2026
4a277a7
Merge branch 'PintaProject:master' into improvement1
Lehonti Apr 9, 2026
4a73d6f
Merged from `master`
Lehonti Apr 20, 2026
72ca35b
Deleted unnecessary code
Lehonti Apr 20, 2026
fdf08ec
Merge branch 'PintaProject:master' into improvement1
Lehonti Apr 25, 2026
500f60e
Removed unused (?) `tools` parameter
Lehonti Apr 25, 2026
594658d
Merge branch 'PintaProject:master' into improvement1
Lehonti May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions Pinta.Core/Actions/EditActions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ private void HandlePintaCoreActionsEditFillSelectionActivated (object sender, Ev
Translations.GetString ("Fill Selection"),
old,
doc.Layers.CurrentUserLayerIndex
)
),
this
);
}

Expand All @@ -341,7 +342,7 @@ private void HandlePintaCoreActionsEditSelectAllActivated (object sender, EventA
doc.ResetSelectionPaths ();
doc.Selection.Visible = true;

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, this);
doc.Workspace.Invalidate ();
}

Expand All @@ -367,7 +368,8 @@ private void HandlePintaCoreActionsEditEraseSelectionActivated (object sender, E
sender switch {
string and "Cut" => new SimpleHistoryItem (Resources.StandardIcons.EditCut, Translations.GetString ("Cut"), old, doc.Layers.CurrentUserLayerIndex),
_ => new SimpleHistoryItem (Resources.Icons.EditSelectionErase, Translations.GetString ("Erase Selection"), old, doc.Layers.CurrentUserLayerIndex),
}
},
this
);
}

Expand All @@ -386,7 +388,7 @@ private void HandlePintaCoreActionsEditDeselectActivated (object sender, EventAr

doc.ResetSelectionPaths ();

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, this);
doc.Workspace.Invalidate ();
}

Expand Down Expand Up @@ -456,7 +458,7 @@ private void HandlerPintaCoreActionsEditUndoActivated (object sender, EventArgs
if (tools.CurrentTool?.DoHandleUndo (doc) == true)
return;

doc.History.Undo ();
doc.History.Undo (this);

tools.CurrentTool?.DoAfterUndo (doc);
}
Expand All @@ -468,7 +470,7 @@ private void HandlerPintaCoreActionsEditRedoActivated (object sender, EventArgs
if (tools.CurrentTool?.DoHandleRedo (doc) == true)
return;

doc.History.Redo ();
doc.History.Redo (this);

tools.CurrentTool?.DoAfterRedo (doc);
}
Expand Down Expand Up @@ -610,7 +612,7 @@ void HandleInvertSelectionActivated (object sender, EventArgs e)

doc.Selection.Invert (doc.ImageSize);

doc.History.PushNewItem (historyItem);
doc.History.PushNewItem (historyItem, this);
doc.Workspace.Invalidate ();
}

Expand Down
19 changes: 11 additions & 8 deletions Pinta.Core/Actions/ImageActions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ public sealed class ImageActions

private readonly ToolManager tools;
private readonly WorkspaceManager workspace;
private readonly EditActions edit;
private readonly ViewActions view;

public ImageActions (
ToolManager tools,
WorkspaceManager workspace,
EditActions edit,
ViewActions view)
{
CropToSelection = new Command (
Expand Down Expand Up @@ -121,6 +123,7 @@ public ImageActions (

this.tools = tools;
this.workspace = workspace;
this.edit = edit;
this.view = view;
}

Expand Down Expand Up @@ -191,7 +194,7 @@ private void HandlePintaCoreActionsImageRotateCCWActivated (object sender, Event
tools.Commit ();

doc.RotateImageCCW ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate90CCW));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate90CCW), edit);
}

private void HandlePintaCoreActionsImageRotateCWActivated (object sender, EventArgs e)
Expand All @@ -201,7 +204,7 @@ private void HandlePintaCoreActionsImageRotateCWActivated (object sender, EventA
tools.Commit ();

doc.RotateImageCW ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate90CW));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate90CW), edit);
}

private void HandlePintaCoreActionsImageFlattenActivated (object sender, EventArgs e)
Expand All @@ -227,7 +230,7 @@ private void HandlePintaCoreActionsImageFlattenActivated (object sender, EventAr

hist.Push (new SimpleHistoryItem (string.Empty, string.Empty, oldBottomSurface, 0));

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsImageRotate180Activated (object sender, EventArgs e)
Expand All @@ -237,7 +240,7 @@ private void HandlePintaCoreActionsImageRotate180Activated (object sender, Event
tools.Commit ();

doc.RotateImage180 ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate180));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.Rotate180), edit);
}

private void HandlePintaCoreActionsImageFlipVerticalActivated (object sender, EventArgs e)
Expand All @@ -247,7 +250,7 @@ private void HandlePintaCoreActionsImageFlipVerticalActivated (object sender, Ev
tools.Commit ();

doc.FlipImageVertical ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipVertical));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipVertical), edit);
}

private void HandlePintaCoreActionsImageFlipHorizontalActivated (object sender, EventArgs e)
Expand All @@ -257,7 +260,7 @@ private void HandlePintaCoreActionsImageFlipHorizontalActivated (object sender,
tools.Commit ();

doc.FlipImageHorizontal ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipHorizontal));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipHorizontal), edit);
}

private void HandlePintaCoreActionsImageCropToSelectionActivated (object sender, EventArgs e)
Expand Down Expand Up @@ -300,7 +303,7 @@ void CropImageToRectangle (Document doc, RectangleI rect, Path? selection)
double original_scale = doc.Workspace.Scale;
doc.ImageSize = rect.Size;
doc.Workspace.ViewSize = rect.Size;
doc.Workspace.Scale = original_scale;
doc.Workspace.SetScale (original_scale);

view.UpdateCanvasScale ();

Expand All @@ -309,7 +312,7 @@ void CropImageToRectangle (Document doc, RectangleI rect, Path? selection)

hist.FinishSnapshotOfImage ();

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
doc.ResetSelectionPaths ();

doc.Workspace.Invalidate ();
Expand Down
21 changes: 12 additions & 9 deletions Pinta.Core/Actions/LayerActions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ public sealed class LayerActions
private readonly RecentFileManager recent_files;
private readonly ToolManager tools;
private readonly WorkspaceManager workspace;
private readonly EditActions edit;
private readonly ImageActions image;
public LayerActions (
ChromeManager chrome,
ImageConverterManager imageFormats,
RecentFileManager recentFiles,
ToolManager tools,
WorkspaceManager workspace,
EditActions edit,
ImageActions image)
{
AddNewLayer = new Command (
Expand Down Expand Up @@ -132,6 +134,7 @@ public LayerActions (
recent_files = recentFiles;
this.tools = tools;
this.workspace = workspace;
this.edit = edit;
this.image = image;
}

Expand Down Expand Up @@ -261,7 +264,7 @@ private async void HandlePintaCoreActionsLayersImportFromFileActivated (object s
// --- Changes to document go after everything else is completed successfully

doc.Layers.SetCurrentUserLayer (layer);
doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
doc.Workspace.Invalidate ();
}

Expand All @@ -273,7 +276,7 @@ private void HandlePintaCoreActionsLayersFlipVerticalActivated (object sender, E

doc.Layers.CurrentUserLayer.FlipVertical ();
doc.Workspace.Invalidate ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipLayerVertical, doc.Layers.CurrentUserLayerIndex));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipLayerVertical, doc.Layers.CurrentUserLayerIndex), edit);
}

private void HandlePintaCoreActionsLayersFlipHorizontalActivated (object sender, EventArgs e)
Expand All @@ -284,7 +287,7 @@ private void HandlePintaCoreActionsLayersFlipHorizontalActivated (object sender,

doc.Layers.CurrentUserLayer.FlipHorizontal ();
doc.Workspace.Invalidate ();
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipLayerHorizontal, doc.Layers.CurrentUserLayerIndex));
doc.History.PushNewItem (new InvertHistoryItem (InvertType.FlipLayerHorizontal, doc.Layers.CurrentUserLayerIndex), edit);
}

private void HandlePintaCoreActionsLayersMoveLayerUpActivated (object sender, EventArgs e)
Expand All @@ -300,7 +303,7 @@ private void HandlePintaCoreActionsLayersMoveLayerUpActivated (object sender, Ev
doc.Layers.CurrentUserLayerIndex + 1);

doc.Layers.MoveCurrentLayerUp ();
doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsLayersMoveLayerDownActivated (object sender, EventArgs e)
Expand All @@ -316,7 +319,7 @@ private void HandlePintaCoreActionsLayersMoveLayerDownActivated (object sender,
doc.Layers.CurrentUserLayerIndex - 1);

doc.Layers.MoveCurrentLayerDown ();
doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsLayersMergeLayerDownActivated (object sender, EventArgs e)
Expand Down Expand Up @@ -348,7 +351,7 @@ private void HandlePintaCoreActionsLayersMergeLayerDownActivated (object sender,
hist.Push (h1);
hist.Push (h2);

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsLayersDuplicateLayerActivated (object sender, EventArgs e)
Expand All @@ -366,7 +369,7 @@ private void HandlePintaCoreActionsLayersDuplicateLayerActivated (object sender,
Resources.Icons.LayerDuplicate,
Translations.GetString ("Duplicate Layer"),
doc.Layers.IndexOf (l));
doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsLayersDeleteLayerActivated (object sender, EventArgs e)
Expand All @@ -383,7 +386,7 @@ private void HandlePintaCoreActionsLayersDeleteLayerActivated (object sender, Ev

doc.Layers.DeleteLayer (doc.Layers.CurrentUserLayerIndex);

doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}

private void HandlePintaCoreActionsLayersAddNewLayerActivated (object sender, EventArgs e)
Expand All @@ -400,6 +403,6 @@ private void HandlePintaCoreActionsLayersAddNewLayerActivated (object sender, Ev
Resources.Icons.LayerNew,
Translations.GetString ("Add New Layer"),
doc.Layers.IndexOf (l));
doc.History.PushNewItem (hist);
doc.History.PushNewItem (hist, edit);
}
}
12 changes: 6 additions & 6 deletions Pinta.Core/Classes/Document.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public void ClearFileReference ()
public void Close ()
{
Layers.Close ();
Workspace.History.Clear ();
Workspace.History.Clear (actions.Edit);
}

public Context CreateClippedContext ()
Expand Down Expand Up @@ -233,7 +233,7 @@ public void FinishSelection ()
Layers.DestroySelectionLayer ();
Workspace.Invalidate ();

Workspace.History.PushNewItem (hist);
Workspace.History.PushNewItem (hist, actions.Edit);
}

// Flip image horizontally
Expand Down Expand Up @@ -341,11 +341,11 @@ public void ResizeCanvas (
if (compoundAction is not null)
compoundAction.Push (hist);
else
Workspace.History.PushNewItem (hist);
Workspace.History.PushNewItem (hist, actions.Edit);

ResetSelectionPaths ();

Workspace.Scale = scale;
Workspace.SetScale (scale);
}

public void ResizeImage (
Expand All @@ -370,11 +370,11 @@ public void ResizeImage (

hist.FinishSnapshotOfImage ();

Workspace.History.PushNewItem (hist);
Workspace.History.PushNewItem (hist, actions.Edit);

ResetSelectionPaths ();

Workspace.Scale = scale;
Workspace.SetScale (scale);
actions.View.UpdateCanvasScale ();
}

Expand Down
15 changes: 5 additions & 10 deletions Pinta.Core/Classes/DocumentHistory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ namespace Pinta.Core;

public sealed class DocumentHistory
{
private readonly EditActions edit;

private readonly Document document;

private readonly List<BaseHistoryItem> history = [];
Expand All @@ -42,11 +40,8 @@ public sealed class DocumentHistory
public event EventHandler? ActionUndone;
public event EventHandler? ActionRedone;

internal DocumentHistory (
EditActions edit,
Document document)
internal DocumentHistory (Document document)
{
this.edit = edit;
this.document = document;
}

Expand All @@ -63,7 +58,7 @@ internal DocumentHistory (

public IEnumerable<BaseHistoryItem> Items => history;

public void PushNewItem (BaseHistoryItem newItem)
public void PushNewItem (BaseHistoryItem newItem, EditActions edit)
Copy link
Copy Markdown
Member

@cameronwhite cameronwhite Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the future fix here would be to avoid the dependency entirely - if the EditActions instead listened for history events (similar to the history pad) then it could update the disabled state of the undo/redo actions without needing the Document to be aware of this

If that seems easy to get working, then maybe we just do that instead of introducing and then removing these parameters? Otherwise I'm okay with the changes as an intermediate step

Copy link
Copy Markdown
Contributor Author

@Lehonti Lehonti Apr 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that could very well be the case, and sorry if the refactoring seems a bit all over the place sometimes, but sometimes things that seem simple, like removing a dependency, end up touching a lot of files, and I have to 'slice' the planned changes to keep the difficulty of the review reasonable (like here...I planned to remove all dependencies from Document but I couldn't even get all of them out of DocumentWorkspace). In this case, Visual Studio reports that PushNewItem has 54 references. I would rather keep the refactoring you are suggesting for a future pull request, and frankly (at a first glance, looking at the call sites) I think it's quite a few intermediate steps away.

Copy link
Copy Markdown
Member

@cameronwhite cameronwhite Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this more, I am a bit concerned with this intermediate step because it changes a commonly-used API for add-ins (for example, the PintaDemoExtension add-in has a BaseTool subclass which creates history items)
So if we don't end up being able to remove this new parameter before the next release, we'll have some unnecessary churn on bumping the add-in API version multiple times

Just to expand more on my idea from the previous comment: EditAcitons already subscribes to the "active document changed" event to update the state of the undo / redo buttons:

Pinta/Pinta.Core/Actions/EditActions.cs

Line 617 in 5cbe79c

private void WorkspaceActiveDocumentChanged (object? sender, EventArgs e)
)
So if this went a bit further and also listened for changes to the document's history stack (like the history panel does:

Pinta/Pinta.Gui.Widgets/Widgets/History/HistoryListView.cs

Line 98 in 5cbe79c

private void OnActiveDocumentChanged (object? sender, EventArgs e)
), then we don't need to pass EditActions into DocumentHistory at all

Copy link
Copy Markdown
Contributor Author

@Lehonti Lehonti Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cameronwhite I understand that concern and that you want to avoid breaking the API. Just for information, I am going through a very busy period, and I don't expect to be able to look into this (which is not just about the code changes, but also troubleshooting my local dev setup) until around the ~15th of May. There is a slim chance that I am able to take a look in the next four days, but even then it would be tough. Or if you wish you can merge this and take care of the remaining part.

Copy link
Copy Markdown
Member

@cameronwhite cameronwhite May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! I might look at making the changes I suggested if I find some time

{
// Remove all old redos starting from the end of the list
for (var i = history.Count - 1; i >= 0; i--) {
Expand All @@ -89,7 +84,7 @@ public void PushNewItem (BaseHistoryItem newItem)
HistoryItemAdded?.Invoke (this, new HistoryItemAddedEventArgs (newItem));
}

public void Undo ()
public void Undo (EditActions edit)
{
if (Pointer < 0)
throw new InvalidOperationException ("Undo stack is empty");
Expand All @@ -114,7 +109,7 @@ public void Undo ()
ActionUndone?.Invoke (this, EventArgs.Empty);
}

public void Redo ()
public void Redo (EditActions edit)
{
if (Pointer >= history.Count - 1)
throw new InvalidOperationException ("Redo stack is empty");
Expand Down Expand Up @@ -160,7 +155,7 @@ public void SetDirty ()
document.IsDirty = true;
}

public void Clear ()
public void Clear (EditActions edit)
{
history.Clear ();
Pointer = -1;
Expand Down
Loading
Loading