Skip to content

Camera: calculate control points on the fly#1738

Merged
J5lx merged 10 commits intopencil2d:masterfrom
MrStevns:camera-tool-painter
Jan 26, 2023
Merged

Camera: calculate control points on the fly#1738
J5lx merged 10 commits intopencil2d:masterfrom
MrStevns:camera-tool-painter

Conversation

@MrStevns
Copy link
Member

@MrStevns MrStevns commented Nov 5, 2022

For this PR the goal has been to create stronger coupling between the camera tool and the painter that handled the interactive parts. This has been done by introducing a BaseTool::paint(...) method which allows us to perform calculations that can both be used by the tool and its painter without addition branching in Scribbearea. One example where we benefit from this is when calculating the centered control point instead of having to explicitly set it at various times. Another example is when calculating the distance between the rotation anchor, its line and the camera frame, as this has to be done two places, locally in the tool and in the world coordinate space.

Fixes

  • Rotation anchor would not keep a fixed distance from the camera frame
    How to reproduce:
  1. Load a project, select the camera tool and the camera layer
  2. Zoom in and out, notice the distance of the rotation anchor being slightly off
  3. Now increase the size of the camera frame and repeat the steps
    Result: The rotation anchor has been positioned further away from the frame or if decreased, position closer to the frame.
  • Sometimes dragging the last control handle would drag the previous instead...

  • Reworked recovery logic for splitting and merging control points

Changes

  • Camera tool painter logic has been moved to camera tool
  • PathCPM has been removed as its not necessary anymore

@MrStevns
Copy link
Member Author

MrStevns commented Nov 5, 2022

Agh, I forgot to update the tests, will do that tomorrow...

- added test for recovering control point on delete keyframe
- added test for preserving curve on add keyframe
@MrStevns
Copy link
Member Author

MrStevns commented Nov 6, 2022

Tests now updated, PR is ready :)

Copy link
Member

@J5lx J5lx left a comment

Choose a reason for hiding this comment

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

LGTM! I like the control point merge logic, and nice refactoring and fixes too. Great job! Will merge once checks have completed.

@J5lx J5lx force-pushed the camera-tool-painter branch from dc46cef to 5c00796 Compare January 26, 2023 23:09
@J5lx
Copy link
Member

J5lx commented Jan 26, 2023

Alright, finally got the build failures sorted out…

@J5lx J5lx merged commit fdb04bd into pencil2d:master Jan 26, 2023
@MrStevns
Copy link
Member Author

Thanks for reviewing, fixing build errors and merging! 💯

@MrStevns MrStevns added this to the v0.6.7 milestone Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

2 participants