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

controls not considered when using perPixelTargetFind if cache canvas is available #5443

Closed
finom opened this issue Dec 25, 2018 · 18 comments · Fixed by #5455
Closed

controls not considered when using perPixelTargetFind if cache canvas is available #5443

finom opened this issue Dec 25, 2018 · 18 comments · Fixed by #5455
Labels

Comments

@finom
Copy link
Contributor

finom commented Dec 25, 2018

Version

2.4.5

Test Case

http://jsfiddle.net/Da7SP/3322/
The test case is quite complicated because I had to transpile the code via babel. But the idea is simple: I want to render custom controls (blue circles) by _renderControls outside of the object (red arrow) itself (at "transparent zone") and use perPixelTargetFind: true.

Information about environment

Chrome 71

Steps to reproduce

Try to use handles (blue circles). First try to mousedown an intersect of a handle and the red arrow. Then try to mousedown the handle outside of the arrow.

Expected Behavior

The handles should be included at "perPixelTargetFind" zone.

Actual Behavior

When a handle is moused down outside of the arrow area, the arrow becomes inactive. It worked fine at 2.0.0-rc.4 if I set padding: "any number more than handle radius" but doesn't work anymore at latest version. The gif above is made using the old version of fabricjs.

@asturur
Copy link
Member

asturur commented Dec 29, 2018

i ll take a look asap.
Do you know that simple round controls are anyway a feature of the lib and you do not need custom code for them?

@finom
Copy link
Contributor Author

finom commented Dec 29, 2018

@asturur thank you!
No, I didn't know. How can I enable them 😊? Can't find any info. The only thing I could do is:

image

@asturur
Copy link
Member

asturur commented Dec 29, 2018

http://fabricjs.com/docs/fabric.Object.html#cornerStyle

start to look from here and here:

http://fabricjs.com/fabric-changelog-old ( scroll down to 1.6.2 )

Also here:

http://fabricjs.com/controls-customization

image

That does not mean your problem is solved, i wanted just to point out that writing code to render circle controls is not necessary.

Said so i could not dig in your fiddle since the transpiled code is too much.
You need to focus on the important part that is target Transparency + custom controls, reducing to the bone the features that are not part of the bug.

How you draw the arrow should not be a problem here, just use a line.

@finom
Copy link
Contributor Author

finom commented Dec 30, 2018

@asturur thank you for the links. My goal isn't to change the styling of standard controls (with 9 handles), my goals is to make completely custom controls with 2 handles on both ends of the arrow (inherited from Line class).

I have simplified the snipped above by removing _render method, most of event handlers and other lines of code. I've left only one handle and one event listener (mousedown). See http://jsfiddle.net/Da7SP/3342/

The problem is when you mousedown a thing rendered by _renderControls, its related object loses itsactive state (if perPixelTargetFind is used). In other words if you render something by _renderControls fabricjs thinks of it as of "transparent" zone.

Let me know is that clearer now.

@asturur
Copy link
Member

asturur commented Dec 30, 2018

Yes, the problem was clear, i wonder how it happens since the standard controls are usable with perPixelTargetFind.

My comment and links about the circle controls is that while you probably need to rewrite the function that render borders and call the function that draw the single controls, the exact function that draw the controls should be fine

@asturur
Copy link
Member

asturur commented Dec 30, 2018

Ok, the control does not work because you set hasControls to false.
You need to draw handle the controls with fabric logic or it won't work.

An easier way to handle this would be:

  • leave just the real controls ml and mr active.
  • make so that at initialization time the line is mad horizontal and rotated to reach the requested points
  • add an hanlder for the scale that will move the points rather than scaling.

@finom
Copy link
Contributor Author

finom commented Dec 30, 2018

the control does not work because you set hasControls to false

I've set the option to true, but it doesn't work anyway. IMO that's another bug: if hasControls is false then _renderControls should not be called or at least its result should be invisible. It appears that hasControls is only handled at the original function. Does it make sense?

An easier way to handle this would be

That looks like a tricky hack... I'm thinking to move controls rendering to _render method depending on its active state or make it as you proposed "you probably need to rewrite the function that render borders and call the function that draw the single controls". I guess you meant drawBorders method?

_renderControls is a part of the official API. I think it should either be removed from the documentation or it should be possible to override it. IMO currently it looks like a private method with public documentation.

@asturur
Copy link
Member

asturur commented Dec 30, 2018

http://jsfiddle.net/Da7SP/3344/
This is an example how to handle it differently ( i did not write the code to move the point of the line around )

There is for sure a bug that stops you from making your code work, and that can you work around disabling objectCaching for your subclass. And the bug should be fixed somehow.

Then my comment about the code is more about the fact that just rendering controls where you want will not make the controls easily targetable in other situations, is also true that i did not read the full code of the first fiddle and i have no idea how other things are handled.

About the trick hack i feel like those are similar way to get to the same result. You should override the simpler functions or the one with which you are most comfortable with if there is more than an option.

There is a connection between oCoords and controls rendering and control radius, you should keep those things as much as working together as possible.

@asturur asturur added the bug label Dec 30, 2018
@asturur asturur changed the title No way to use custom controls rendered with _renderControls if perPixelTargetFind is true controls not considered when using perPixelTargetFind if cache canvas is available Dec 30, 2018
@finom
Copy link
Contributor Author

finom commented Dec 30, 2018

@asturur thank you.

@asturur
Copy link
Member

asturur commented Dec 30, 2018

i m fixing the bug we surfaced, this may fix or not fix your use case, i have no idea.

@finom
Copy link
Contributor Author

finom commented Jan 4, 2019

@asturur any news about an official release?

@asturur
Copy link
Member

asturur commented Jan 6, 2019

i think i ll go for this sunday now that the gradient export is in too.

@Hellowor1d
Copy link

Version

2.4.5

Test Case

http://jsfiddle.net/Da7SP/3322/

This is a wonderful drag effect about the arrowLine, wiil you please update it after asturur's method in http://jsfiddle.net/Da7SP/3344/ ?

@finom
Copy link
Contributor Author

finom commented Jan 6, 2019

@Hellowor1d a company I work for is planning to open source an ambitious image editor (but not earlier than in a month). Line arrow class is going to be open sourced as well.

@Hellowor1d
Copy link

Hellowor1d commented Jan 27, 2019

@Hellowor1d a company I work for is planning to open source an ambitious image editor (but not earlier than in a month). Line arrow class is going to be open sourced as well.

Hi~ , has been the ambitious image editor published? waiting for your good news

@finom
Copy link
Contributor Author

finom commented Aug 26, 2019

@Hellowor1d not yet :(

@thanh-taro
Copy link

thanh-taro commented Jul 5, 2023

I faced the same issue (maybe), and in my case, it is because of the cornerSize of default controls (the 9 controls).
If it set to 10px then your controls also have 10px for clickable area and totally ignoring the cornerSize you've set when create the custom controls. Click outside of it will deactivate the object and nothing happens.

And I have no idea how to fix it. Tried somethings but still doesn't work.

@thanh-taro
Copy link

Figured it out!
You have to set all of below when create a custom control:

  cornerSize: 32,
  touchCornerSize: 32,
  sizeX: 32,
  sizeY: 32,
  touchSizeX: 32,
  touchSizeY: 32,

32 is my control's size, replace with yours.

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

Successfully merging a pull request may close this issue.

4 participants