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.

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

Circle layers have wrong selection area #442

Closed
zozorg opened this Issue Oct 5, 2017 · 5 comments

Comments

3 participants
@zozorg

zozorg commented Oct 5, 2017

Synfig version & platform:
linux 1.3.4

Issue description:
A circle layer can be selected by clicking on its right at any distance. It makes circles of low Z depth so annoying you have to deactivate them.

@zeograd

This comment has been minimized.

zeograd commented Oct 15, 2017

It seems like CurveArray::intersect(Real, Real, Point*) (in layer_shape.cpp:525)
starts with

	if((y < aabb.miny) || (y > aabb.maxy) || (x < aabb.minx)) return 0;

while it should rather be

	if((y < aabb.miny) || (y > aabb.maxy) || (x < aabb.minx) || (x > aabb.maxx)) return 0;

Note that doing so still allow to incorrectly select a circle in the 2 right areas between the actual curve and its AABB due to intersect_conic matching twice in the special degenerate accept case in layer_shape.cpp:241

	//degenerate accept - to the right and crossing the base line
	if(x > xmax)
	{
		return (y <= ymax && y >= ymin);
	}

The fact that the degenerate accept case of intersect_conic matches for x > xmax makes sense that the calling intersect function did NOT checked x vs aabb.maxx but I don't understand the usefulness of the degenerate accept case for now (probably too broad as it is written now)

morevnaproject added a commit to morevnaproject/synfig that referenced this issue Oct 20, 2017

@morevnaproject

This comment has been minimized.

Member

morevnaproject commented Oct 20, 2017

Thank you @zeograd! This is already better than before! I have committed your fix.

@zeograd

This comment has been minimized.

zeograd commented Oct 20, 2017

You're welcome. I hoped I could do better but digging into a 12 years old function filled by conic maths without a good knowledge of the rest of synfig was a bit daunting.

@morevnaproject morevnaproject added this to In Progress in Releases Oct 26, 2017

morevnaproject added a commit to morevnaproject/synfig that referenced this issue Oct 26, 2017

Revert "Parital fix for synfig#442 - Circle layers have wrong selecti…
…on area. Thanks to Olivier Jolly (@zeograd)."

This reverts commit c3872b8.
@morevnaproject

This comment has been minimized.

Member

morevnaproject commented Oct 26, 2017

I found that this bug was introduced in c6f7457 and 2fbdbcf when Circle Layer was rewritten to use Layer Shape. For some reason Layer Shape doesn't parse hit_check() function for this case correctly.

I have added explicit hit_check() function as a workaround. Closed via 38c383b.

@morevnaproject morevnaproject moved this from In Progress to Done in Releases Oct 26, 2017

@morevnaproject morevnaproject moved this from Done to Released in Releases Dec 25, 2017

@morevnaproject

This comment has been minimized.

Member

morevnaproject commented Dec 25, 2017

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