Skip to content

endpoint adjustment framework - #1232

Open
AlertCoffee wants to merge 2 commits into
LinwoodDev:developfrom
AlertCoffee:endpoint-adjustment
Open

AlertCoffee wants to merge 2 commits into
LinwoodDev:developfrom
AlertCoffee:endpoint-adjustment

Conversation

@AlertCoffee

Copy link
Copy Markdown

Added a feature for adjusting the endpoints of lines made with the "detect shape" tool.

This was tested using the chrome version VM.

Keyboard and (mainly) mouse worked fine, no bugs that i've been able to catch.

Stylus and tablet was tested with a Wacom Intuous Pro. A bug I noticed and haven't been able to fix is that after a line is drawn, you have to pause the stroke, then draw a smaller stroke for it to register as a shape.

@CodeDoctorDE CodeDoctorDE left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for contributing. I have some things found i would change.

Additionally please run dart format . to format the project.

Comment thread app/lib/handlers/pen.dart
String? _pendingLineCollection;

// Pointer index for the pointer which triggered
int? _adjustingPointer;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The current pen handler has many variables currently. I'm thinking if we can improve it by structuring it better?

Comment thread app/lib/handlers/pen.dart
? PenRenderer(
e.copyWith(
id: createUniqueId(),
points: limitPenPreviewPoints(e.points),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I removed the limitPenPreviewPoints, can you just remove this line?

Comment thread app/lib/handlers/pen.dart
lastPosition.clear();
points.clear();
lastPosit = null;
// NEW: Reset line adjustment state

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This drops an already detected pending line when the input is reset, e.g. when switching tools. Could we commit the pending line before clearing the adjustment state?

Comment thread app/lib/handlers/pen.dart
_hideCursorWhileDrawing = context.getSettings().hideCursorWhileDrawing;

// If a new pointer is detected, commits the line which is being adjusted
if (_isAdjustingLine) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the adjusting pointer gets cancelled, this state can survive until the next stroke, which then commits the stale line here. Could we handle this explicitly in onPointerCancel?

@CodeDoctorDE

Copy link
Copy Markdown
Member

Hey, just wanted to check in: how is it looking with this PR?

I'm slowly getting ready to release Butterfly 2.6, so it would be great to know if you still plan to address the review comments and get this ready for merging before the release. No pressure, I just want to get a better idea of the current status 馃檪

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants