isPointInside incorrectly caches the path used to calculate its value #596

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@lvanderbijl

isPointInside() caches "this.realPath" var to calculate a return. This means if you modified a shapes attributes using the .attr() method (as opposed to using a transform) this meant the real path used would be wrong.

My change forces isPointInside to always re-calculate the realPath.

Looking through the code, it actually looks like realPath is only used as a local variable in a few methods - and not as a cached "global" variable. It might be worth removing it entirely - or alternatively create a fully featured global to cache the realPath correctly (ie update it every time the shapes' properties change).

isPointInside() used to use the cached "realPath" var to calculate a …
…return. This meant if you modified a shapes attributes using the .attr() method (as opposed to using a transform) this meant the real path used would be wrong.


This forces isPointInside to always re-calculate the realPath.

Looking through the code, it actually looks like realPath is only used as a local variable in a few methods - and not as a cached "global" variable. It might be worth removing it entirely - or alternatively create a full feature global to cache the realPath.
@aishwar

This comment has been minimized.

Show comment Hide comment
@aishwar

aishwar Mar 1, 2013

I just ran into this issue myself. I created glow for an element. I updated it using ".attr('x', ...)". I removed the old glow and called glow again to update it. It drew the glow in the old location. And this was because of the bad cached value at "this.realPath".

This will also cause future calls to isPointInside to fail!

aishwar commented on 7300ab0 Mar 1, 2013

I just ran into this issue myself. I created glow for an element. I updated it using ".attr('x', ...)". I removed the old glow and called glow again to update it. It drew the glow in the old location. And this was because of the bad cached value at "this.realPath".

This will also cause future calls to isPointInside to fail!

@tomasAlabes

This comment has been minimized.

Show comment Hide comment
@tomasAlabes

tomasAlabes Jun 2, 2013

Collaborator

Pushed in v2.1.1 branch. Thanks.

Collaborator

tomasAlabes commented Jun 2, 2013

Pushed in v2.1.1 branch. Thanks.

@tomasAlabes tomasAlabes closed this Jun 2, 2013

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