#2029: fix regression bug where cursor was no longer updated on zoom

[spaghetti22]

Hi, here's the fix for the bug I unfortunately introduced and didn't notice.

[cameronwhite]

Thanks for looking into this!

Reading through the code I'm not really a big fan of how things originally worked with this CursorChangesOnZoom property, so this would be a good opportunity to clean that up rather than patching it further

The CursorChangesOnZoom property isn't great for a few reasons:

• It relies on the DefaultCursor property returning a different cursor instance if the zoom has changed, without otherwise notifying the tool about the change. This was something you were already starting to change in this PR 👍
  • Even worse, the fact that the cursor changes on zoom is hidden deep inside CreateIconWithShape which accesses the global PintaCore.Workspace.Scale.
• The DocumentWorkspace has to have a reference to the ToolManager to do this, which isn't ideal since this is one of the innermost classes in the dependency chain and shouldn't depend on tools (instead, tools depend on the document)
• Also, there already is a ViewSizeChanged event that could be used to listen for changes to the zoom instead of adding this to the BaseTool API

These are the changes I think we could make:

• Remove the CursorChangesOnZoom property. Tools instead subscribe to the workspace's ViewSizeChanged event if they need a notification about the zoom changing . This is the same idea as how the SelectTool listens for changes to the document's selection
  • This event could be "promoted" to IWorkspaceService / WorkspaceManager like what's done for the LayerAdded event so that tools can subscribe to an event for whatever the current document is, without worrying about handling documents being added / removed
  • The tools member should be able to be deleted from DocumentWorkspace
• Add a scale parameter to CreateIconWithShape()

[spaghetti22]

I implemented the changes as you requested them, except that I didn't add a scale parameter to CreateIconWithShape, which struck me as unnecessary because it would always be called with the same value and this would make the code unnecessarily complex.

I also fixed two bugs:

• the one mentioned in Slash brush #1974 where the correction of painting operations with very small area caused the angle to be slightly off at the start and end
• another one I noticed where at high zoom levels the slash brush cursor was no longer as intended

[cameronwhite]

Thanks for the changes!

except that I didn't add a scale parameter to CreateIconWithShape, which struck me as unnecessary because it would always be called with the same value

I think that value should be changing unless I was misunderstanding something as I read the code. If the canvas is zoomed, then the sequence would be

• workspace sends out a ViewSizeChanged event and the tool decides to update its cursor
• this eventually calls CreateIconWithShape
• the local zoom variable is computed from the active document's workspace Scale which should have changed already to reflect the zoom percentage. This is going through the global PintaCore.Workspace currently and is what I was thinking should be passed in to eliminate the global

[spaghetti22]

I think that value should be changing unless I was misunderstanding something as I read the code. If the canvas is zoomed, then the sequence would be

OK, I see what you mean. I didn't mean that it was always the same (absolute) value, merely that the value would always come from the same source. I've now changed it the way you requested.

[cameronwhite]

Not sure if I'm missing something from just reading the code, but shouldn't workspace always be non-null here?

This block seems to be the most annoying bit of code duplication, so I think adding a helper method in WorkspaceServiceExtensions could be justified.

The changes are looking great otherwise!