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

project hitTest should return all results #536

Closed
Skalkaz opened this issue Oct 14, 2014 · 22 comments
Closed

project hitTest should return all results #536

Skalkaz opened this issue Oct 14, 2014 · 22 comments

Comments

@Skalkaz
Copy link

Skalkaz commented Oct 14, 2014

HitResult contains information about one item of the all possibles. It would be great the all, for example hitResult.items array

@lexar
Copy link

lexar commented Nov 16, 2014

I have the same problem. I have several paths of one kind, each of them hits the same items of another kind. All of the paths of the first kind return the same item of the second kind on a hitTest. Is there any way to get all items which have been hit?
I was thinking about marking each hit item as locked, then recursively performing another hitTest, until there is no item left. Would that be a good idea?

@Skalkaz
Copy link
Author

Skalkaz commented Nov 16, 2014

I did similar thing as workaround.

@lehni
Copy link
Member

lehni commented Nov 17, 2014

I will have to implement this. The workaround works, but is very inefficient, computing-wise. There is an internal requirement for similar functionality for an upcoming refactoring / improvement of how mouse events are handled, and I'll include this there. Keeping the bug open for the time being.

@piotrwitek
Copy link

+1 for events propagating down and hitTest returning array refactor, I was also forced to use them ugly workarounds :/, these events are a highly limiting factor right now.
Thanks for sharing your plans on improvement.

@Finu
Copy link

Finu commented Jul 31, 2015

Hi.
I have two paths. One represents terrain and other - road. Click on the road should select land which is below road, Road should have no interactions or event - just render.
I'm not sure if it's helps any one - or does it have any grave consequences for performace of paper.js, or there is other proper mechanic for this - but I override function isEmpty of road path to return false. For this i can invoke event on path below.
It's just for disabling hit test for certain Item.

Solving this issue would be nice improvement

@ghost
Copy link

ghost commented Aug 1, 2015

👍

@kaelumania
Copy link

+1 Maybe another workaround is to use getItems(match) with match.overlapping: Rectangle by using a very small rectangle.

@lehni
Copy link
Member

lehni commented Jan 12, 2016

@piotrwitek I just realized that events propagating and hitTest() returning all items is no related. Events propagating has to do with them going up the chain of parents, until one of them stops the event (bubbling). For event handling, it is enough for hitTest() to only return the first, top-most item, and then bubble up the event from there. That's also how HTML is handling it.

But I will implement a way to get multiple hit-results in one call. It will not be through hitResult.items as a hit-result object by definition describes the result on only one item. I need to think of a good way to implement this, still.

@lehni lehni added this to the v0.10.0 milestone Jan 13, 2016
@piotrwitek
Copy link

@lehni thanks for your time looking into this issue, this was some time ago, but I'm sure what I meant here was related to only one point, to get multiple hit-result in one call, and probably I didn't express myself clear enough describing it.
What you said in the last paragraph, to add another method returning multiple hit-result, is exactly the point of this issue.

@lehni
Copy link
Member

lehni commented Feb 13, 2016

So I've been thinking about how to implement this in Item#hitTest() and Project#hitTest(). Here a proposal:

  • There will be a new options.matchoption that can be set to a callback function. For each hit, this callback is called with the HitResult object as the argument.
  • If the callback returns true, the hit-test is chosen and returned from the hit-test call.
  • If the callback returns false, or nothing, hitTest() keeps searching for a matching hit-result.

This means:

  • hitTest() will keep returning only one HitResult object
  • If you want to get multiple hits, then you have to collect them yourself in the callback, and keep returning false

If this is too complicated, we can consider a second version, for example

  • Item#hitTestAll() and Project#hitTestAll()
  • options.match is supported, and all results for which it returns true are collected in an array, and returned in the end. If no options.match is defined, all hits are returned.

It could also be named #hitTestMultiple(), or #hitTests(), but I think #hitTestAll() is good.

Thoughts?

@piotrwitek
Copy link

@lehni I think both of your proposal would be useful to add into paper.js, but in my opinion they serve different purposes, and first solution is kinda hackish when you'd like to solve the problem of returning all hitTest items.

First solution would be useful to bring the possibility to integrate custom or some intricate selection logic in hitTest calls, that would not fit into the standard options.match object.

Second proposal is a fitting solution for discussed problem, though I like #hitTestMultiple() name, as it gives a lot of meaning to what it actually do, second would be hitTestAll (NB: hitTests is really weird).

@lehni
Copy link
Member

lehni commented Feb 14, 2016

Thanks for the feedback! I think I prefer 'All', because it returns all hits in that location, not just multiples.

@piotrwitek
Copy link

Thinking again I agree with you, "hit test all" of them seems better.

Pozdrawiam,
Piotr Witek

2016-02-14 10:46 GMT+01:00 Jürg Lehni notifications@github.com:

Thanks for the feedback! I think I prefer 'All', because it returns all
hits in that location, not just multiples.


Reply to this email directly or view it on GitHub
#536 (comment).

@lehni lehni closed this as completed in 4a94731 Feb 14, 2016
@lehni
Copy link
Member

lehni commented Feb 14, 2016

Finally done :)

@StratusBase
Copy link

StratusBase commented Feb 6, 2017

It appears that hitTestAll is not available as a function on any item that I have attempted to call it on... Is something broken?

My goal was to target multiple class items, but only one class can be specified in the options config. I tried adding a match function, but it never gets called...

UPDATE: I just globally scoped my project and checked the available methods for "hit", and "hitTest" is the only available one. Thus, something isn't getting properly injected, or the documentation isn't up to date. After reviewing the Git source, it looks like it's injected... But it does not appear to be in my project...

@lehni
Copy link
Member

lehni commented Feb 6, 2017

You need a develop version for now:

http://paperjs.org/download/#development-builds

@StratusBase
Copy link

StratusBase commented Feb 6, 2017

Ok, so I have pulled / replaced my master file with the current script located here:

https://raw.githubusercontent.com/paperjs/paper.js/prebuilt/module/dist/paper-core.js

Still, the only available "hit" method on the project is "hitTest", nothing more...

@lehni
Copy link
Member

lehni commented Feb 6, 2017

Are you sure? The docs may be outdated...

screen shot 2017-02-06 at 18 33 52

lehni added a commit that referenced this issue Feb 6, 2017
@StratusBase
Copy link

StratusBase commented Feb 6, 2017

I'd be happy to start a Google Hangout and shoot you an invite so that you can see what I am doing if you'd like? To say that I am 100% sure would be wrong, I can only relay what I am seeing in my own project...

@lehni
Copy link
Member

lehni commented Feb 6, 2017

Sounds like a caching issue then... Make sure the new file is actually loaded in your browser. I am on the road right now, not available for hangout.

@StratusBase
Copy link

I assumed that too, I did a "Empty Cache and Hard Reload" in Chrome just to be sure, with the same result. Honestly it's not a huge deal as I can work around it... Just wanted to bring it to your attention.

@lehni
Copy link
Member

lehni commented Feb 7, 2017

Well, it's present here, so I am pretty sure you must be somehow using the wrong version... What do you see when you log this:

console.log(project.hitTestAll);

or

console.log(paper.Project.prototype.hitTestAll);

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

No branches or pull requests

7 participants