Skip to content
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

Selection and cursor leaves trace when zooming #5043

Closed
Tsourdox opened this issue Jun 14, 2018 · 13 comments · Fixed by #5048
Closed

Selection and cursor leaves trace when zooming #5043

Tsourdox opened this issue Jun 14, 2018 · 13 comments · Fixed by #5048

Comments

@Tsourdox
Copy link

Using fabric v2.3.x

Test Case

http://jsfiddle.net/n2yLv0h9/8/

Steps to reproduce

Create a textbox, select the text or click to enable editing with flashing cursor. Then zoom(fast) the canvas.

Expected Behavior

Selection/cursor is re-drawn within the textbox.

Actual Behavior

Selection/cursor leaves a trace outside of the textbox.

Image

skarmavbild 2018-06-14 kl 10 58 13

@samael205
Copy link

**Hint ** This only happens when you select a character that is attached to the selection box.

@Tsourdox
Copy link
Author

@samael205 what do your mean?

@samael205
Copy link

samael205 commented Jun 14, 2018

It only occurs in those selections that involve an edge. As you see in the image "then" is attached to the bounding-box of the object
fabricerror

fabricerror2

@Tsourdox
Copy link
Author

@samael205 Not true, scroll faster and it will happen for selected words in the middle as well (or even if the cursor is in the middle)

@samael205
Copy link

samael205 commented Jun 14, 2018

scrollfast2
scrollfast

Scrolling fast at my home.

That is what i see

I'm just giving my point of view, to help

@asturur
Copy link
Member

asturur commented Jun 16, 2018

Yes this is a problem, i noticed it too.

So the Itext does not clear all the upper canvas to save on speed. It clears the part where it is.
If you scroll before the cleaning happens this is what you see.

I think at least setZoom ( and setViewportTansform ) should take care of cleaning before reRendering

@Tsourdox
Copy link
Author

Wow, that was fast! thx 🙏🏅

@eldstrom
Copy link

eldstrom commented Jul 27, 2020

@asturur Still having this issue. Was this fix removed? I tried to override setViewportTansform and apply it again on the static canvas since it was only there on the normal canvas but i still have this problem.
Not on scroll tho, only on ZoomToPoint.

@asturur
Copy link
Member

asturur commented Jul 27, 2020

no that i know of

@eldstrom
Copy link

We had set renderOnAddRemove to false so the fix was never hit. Changed that and it now looks as it should so the fix still works.
I did try to do the clearContextTop and requestRenderAll in my own code but it had no effect there, not sure why but i will try this now and see if it causes any issues for us.

@asturur
Copy link
Member

asturur commented Jul 27, 2020

i would expect renderOnAddRemove to not interfere with the fix. Why it does? can you help me understand?

@eldstrom
Copy link

eldstrom commented Jul 27, 2020

The only code that was added in setViewportTransform in canvas.class.js that does something is this code:

if (this.renderOnAddRemove && this._activeObject && this._activeObject.isEditing) { this._activeObject.clearContextTop(); }

As i understand it, it is clearContextTop method that clears the marked text + cursor trail before each re-render. The if statement here requires renderOnAddRemove to be true.

In static_canvas.class.js on line 698 you have this line:
this.renderOnAddRemove && this.requestRenderAll();
Now, my js is a bit bad but to me that looks like also there, renderOnAddRemove is required to run requestRenderAll.

@asturur
Copy link
Member

asturur commented Jul 28, 2020

that is weird! can you open a separate bug ticket so i can track it? Is also important that you make a jsfiddle that makes the problem actually happen so that it can be used to see if the fix is effective.

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

Successfully merging a pull request may close this issue.

4 participants