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

Collision boxes don't account for symbol-placement:point/rotation-alignment: map #4861

Open
ChrisLoer opened this issue Jun 21, 2017 · 5 comments
Labels

Comments

@ChrisLoer
Copy link
Contributor

As far as I can tell this has always been the case:

screenshot 2017-06-21 13 30 56

Labels with rotation-alignment: map are usually more likely to use symbol-placement: line, and they're mostly fine because collision detection is done with a series of small boxes (icons with lopsided aspect ratios might be an exception here).

This would be straightforward to fix if we moved to an approach of global collision detection using current camera parameters (discussed somewhat in #4704).

/cc @ansis

@ChrisLoer
Copy link
Contributor Author

/cc @mollymerp

@ChrisLoer
Copy link
Contributor Author

This would be straightforward to fix if we moved to an approach of global collision detection using current camera parameters (discussed somewhat in #4704).

It turns out not to be totally straightforward to fix with global collision detection because we still don't have an easy/fast way to represent rectangular geometries that aren't viewport-axis-aligned.

For text line labels, we've started representing their geometry with a series of circles (which have the advantage of keeping the same shape during rotation operations) -- it's possible we could solve this problem by breaking map-aligned rectangles (whether for text or icons) into a series of circles. However, we're not tackling that problem yet in the global collision PR #5150).

/cc @nickidlugash

ChrisLoer added a commit that referenced this issue Aug 17, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 5, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 15, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 22, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 27, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 28, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 29, 2017
…ust use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes.

This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Sep 29, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Oct 3, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Oct 5, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Oct 6, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Oct 17, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
ChrisLoer added a commit that referenced this issue Oct 18, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR #1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue #4861).
VictorHom pushed a commit to VictorHom/mapbox-gl-js that referenced this issue Oct 27, 2017
 - Collision detection now runs on the foreground in a global, viewport-aligned CollisionIndex instead of in the background on map-aligned CollisionTiles.
 - Calculating label size at render time means it's no longer necessary to approximate perspective effects
 - Splits symbol_bucket#prepare and symbol_bucket#place into separate files.
 - Background processing of symbol_bucket, including buffer generation, is now completed with 'prepare'
 - 'place' is now called on the foreground in preparation for render calls.
 - Placement now updates dynamic opacity buffers instead of re-generating large static buffers
 - Line label projection code from 'projection.js' is now used during Placement to calculate the extent of line labels
 - Vertical and horizontal versions of glyphs are now generated at same time in the background. At render time, one version will be hidden based on line angle.
 - Rolls back PR mapbox#1981: even when icons are rotation-aligned to a line, just use a single collision box for them, instead of trying to approximate the shape of the icon with multiple collision boxes. This makes the behavior similar to point icons with rotation-alignment map, which depend on having a roughly square shape (see issue mapbox#4861).
@ChrisLoer
Copy link
Contributor Author

When we fix this, we should also switch icons with symbol-placement: line to use the same behavior (with rotation alignment based on the tangent to the line). We attempted to do that in the past with #1981, but removed that behavior as part of #5150.

@ChrisLoer
Copy link
Contributor Author

Issue #6075 got me experimenting with using the viewport-aligned envelope of the map-aligned collision box. It's pretty easy to do at collision detection time in placeCollisionBox:

        if (rotate) {
            const tl = new Point(tlX, tlY);
            const tr = new Point(brX, tlY);
            const bl = new Point(tlX, brY);
            const br = new Point(brX, brY);
            const center = new Point((brX + tlX) / 2, (brY + tlY) / 2);

            tl._rotateAround(rotate, center);
            tr._rotateAround(rotate, center);
            bl._rotateAround(rotate, center);
            br._rotateAround(rotate, center);

            // Collision features require an "on-axis" geometry,
            // so take the envelope of the rotated geometry
            // (may be quite large for wide labels rotated 45 degrees)
            tlX = Math.min(tl.x, tr.x, bl.x, br.x);
            brX = Math.max(tl.x, tr.x, bl.x, br.x);
            tlY = Math.min(tl.y, tr.y, bl.y, br.y);
            brY = Math.max(tl.y, tr.y, bl.y, br.y);
        }

Implementing the right collision debug behavior is more annoying, however, in part because icons and text both use the same buffers to store debug collision boxes, but icon-rotation-alignment and text-rotation-alignment could have different values.

@smerchek
Copy link

I just ran into this issue. We use a symbol rotated in this way as a hover/click target and it's very disorienting for users once the map is rotated, because they can only click on a small part of the symbol (since the rest of the click target is below the symbol).

Curious if there are any workarounds or fix in sight?

I thought maybe I could simulate a point placement but with a small LineString since symbol-placement=line works with map rotation, but it doesn't really give me the precision I need (and there is also a bug preventing this workaround since the symbol will easily collide #9715).

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

No branches or pull requests

3 participants