make small object on top of active one selectable with preserve object stacking true

[thachnv]

Version

1.6.3

Test Case

http://jsfiddle.net/ecvctazk/

Steps to reproduce

Try to select the inner object

Expected Behavior

You should be able to select the inner object

Actual Behavior

You always got the background object selected

[thachnv]

This problem does not happen in <= 1.6.2 version

[asturur]

I have some difficulties trying to accomodate any selecting situatuon. That involves object selected is under another, but is bigger, or smaller than the covering one. Or when the selecte object has just one handle under another object.

If you think to have a logic that fix all necessity please illustrate it and help me.

[virror]

This does not really seem like the same issue that you just closed for me, but here are my 5 cents anyways. This order should be the most logical one:
-Widgets should always have priority.
-Smaller objects should get selected before larger ones
Not sure the z-index should affect selecting other than if no one of the other conditions are true. Might add an option though that always selects by z-order, but even there, widgets should always have priority.

[butch2k]

I would say the first object to get the click event should keep it. If clicked within its visible boundaries an object get selected. Same logic applies to handles an handle should get the click event as if the object. An handle in front of an object should get the click and keep it. 

On Sun, Jul 10, 2016 at 11:02 PM +0200, "Andrea Bogazzi" notifications@github.com wrote:

I have some difficulties trying to accomodate any selecting situatuon. That involves object selected is under another, but is bigger, or smaller than the covering one. Or when the selecte object has just one handle under another object.

If you think to have a logic that fix all necessity please illustrate it and help me.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[virror]

BTw, my issue occurs on earlier version as well, not just 1.6.2 as for the OP. In our application we changed how selection is handled and smaller objects are always selected regardless of whats above it, like this:
Fabric.Canvas.prototype._searchPossibleTargets = (objects, pointer) ->
target = undefined

for object in objects when @_checkTarget(pointer, object)
    if not target?
        target = object
    else if object.height * object.width < target.height * target.width
        target = object

@relatedTarget = target

This is of course not a complete solution, but i think the concept is much more logical for most cases. To this it also needs to be added a fix for always selecting widgets first, but i have no clue on how that stuff works.

[asturur]

-Widgets should always have priority. Done with #3111

[neopheus]

With the new version of fabricjs : https://jsfiddle.net/ecvctazk/5/

It's better

But we must unselect objects before the select object

[asturur]

The idea for now would be this:

    findTarget: function (e, skipGroup) {
      if (this.skipTargetFind) {
        return;
      }

      var ignoreZoom = true,
          pointer = this.getPointer(e, ignoreZoom),
          activeGroup = this.getActiveGroup(),
          activeObject = this.getActiveObject(),
          firstTarget, secondTarget;
      this.targets = [ ];

      // first check current group (if one exists)
      // active group does not check sub targets like normal groups.
      // if active group just exits.
      if (activeGroup && !skipGroup && this._checkTarget(pointer, activeGroup)) {
        firstTarget = activeGroup;
      }
      // this will take care of subTargets of activeObject if activeObject is a group
      if (activeObject && this._searchPossibleTargets([activeObject], pointer) === activeObject) {
        firstTarget = activeObject;
      }
      if (!this.preserveObjectStacking && firstTarget) {
        return firstTarget;
      }

      var secondTarget = this._searchPossibleTargets(this._objects, pointer, firstTarget);
      if (secondTarget && secondTarget !== firstTarget) {
        if (secondTarget.area() < firstTarget.area() / 2) {
          firstTarget = secondTarget;
        }
      }
      this._fireOverOutEvents(firstTarget, e);
      return firstTarget;
    },

with searchPossibleTargets

    _searchPossibleTargets: function(objects, pointer, firstTarget) {

      // Cache all targets where their bounding box contains point.
      var target, i = objects.length, normalizedPointer, subTarget, _target
      // Do not check for currently grouped objects, since we check the parent group itself.
      // untill we call this function specifically to search inside the activeGroup
      while (i--) {
        _target = objects[i];
        if (_target !== firstTarget && this._checkTarget(pointer, _target)) {
          target = _target;
          if (target.type === 'group' && target.subTargetCheck) {
            normalizedPointer = this._normalizePointer(target, pointer);
            subTarget = this._searchPossibleTargets(target._objects, normalizedPointer);
            subTarget && this.targets.push(subTarget);
          }
          break;
        }
      }
      return target;
    },

We check for activeGroup or activeObject. If activeObject is a group (not activeGroup) we perform subTargetCheck.
Once we are done, if we are not in preserveObjectStacking mode and the target is found, exit.

If we are in preserveObjectStacking, let's see if there is another target in the way that is not activeObject. If there is and is smaller of the current object ( .area() still to create ) then that may be the real target. ( ops need to check also zIndex ).

What do you think?

[asturur]

To check better we are not firing out events for active target or active group now.

[virror]

Sounds sane to me, a bit hard to tell without testing though. But I think it will work fine.

[neopheus]

Like this : https://jsfiddle.net/ecvctazk/9/ ?

[virror]

That fiddle does not work well at all for me. (tested on my phone)

[neopheus]

Yes, a problem with :
(index):78 Uncaught TypeError: secondTarget.area is not a function

[neopheus]

Try this : https://jsfiddle.net/ecvctazk/15/

I disable area

[neopheus]

It's ok

But with multiple object selection there are some problems

[neopheus]

A solution : https://jsfiddle.net/ecvctazk/33/

[virror]

Now nothing works at all.

[asturur]

mine was an idea, i did not get ready the .area function, nor i m sure is a good idea. it can be an improvement over the old one, but too many flags to have a definitive one.

[virror]

I think the current functionality are great, just the issue that you have to unselect before selecting again if the new selected object are above.

[neopheus]

Try this: https://jsfiddle.net/ecvctazk/33/

Now, it's ok for me (like 1.6.2)

[virror]

This works as expected : )

[neopheus]

With the new build : https://jsfiddle.net/ecvctazk/49/

[pavloKozlov]

It's really annoying bug and our team needs a fix for it.
Could you merge pull request above ASAP? It works like a charm!

[asturur]

i m not adding another flag. feel free to take that code and port in the library. As soon as i have some time i will search for a solution that may work good with both preserve object stacking tru and false.

[virror]

Can you put up a fiddle for this?

[asturur]

you mean with new code?

[virror]

Yeah, so i can test it out : )

[asturur]

I think i ll not do it but just because with all the little time i have and the effort i m putting, the time spent in making a fiddle is better spent in fixing a bug or cleaning up a feature.
Sorry Victor.