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

Patterns computed from zoom-based expressions render with incorrect scale #10033

Closed
rf- opened this issue Oct 15, 2020 · 2 comments · Fixed by #10284
Closed

Patterns computed from zoom-based expressions render with incorrect scale #10033

rf- opened this issue Oct 15, 2020 · 2 comments · Fixed by #10284
Assignees
Labels
Milestone

Comments

@rf-
Copy link

rf- commented Oct 15, 2020

It seems like v1.9.1 introduced a bug in the rendering of fill-pattern properties computed from zoom-based step expressions with non-literal values. At least that's as far as I've been able to boil down the repro case so far.

PR #9482 seems potentially relevant, although I don't know enough to understand what the connection is.

mapbox-gl-js version: v1.9.1 and above

browser: Chrome 86.0.4240.75 on macOS 10.15.7

Steps to Trigger Behavior

  1. In the given CodePen, slowly zoom in through a few whole-numbered zoom levels. Note that, once you've gone from the outermost zoom level to the second-outermost, the gray boxes maintain a constant scale relative to the map as you continue zooming in.
  2. Slowly zoom out and note that as soon as you hit a new zoom level, the gray boxes get much larger. As you continue to zoom out all the way, note that they stay this new size at every zoom level.

Link to Demonstration

Expected Behavior

The scale of the pattern within a given zoom level doesn't depend on which direction you're zooming when you enter it.

Actual Behavior

The pattern appears much larger when zooming out than when zooming in.

@rf-
Copy link
Author

rf- commented Oct 22, 2020

It's also worth noting that, when zooming in and out a bunch on any of these examples (the last one, on the newest Mapbox with literal step values, is probably the most relevant), the texture often renders incorrectly and then fixes itself on the next zoom-level transition.

Example of corrupted rendering:
image

Example of correct rendering at the same zoom level:
image

I don't know whether this is related, but it's characteristic of a bunch of random rendering glitches I've run into when trying to find workarounds to allow us to continue rendering consistently-scaled textures like this.

@karimnaaji karimnaaji added this to the v2.1.0 milestone Jan 4, 2021
karimnaaji added a commit that referenced this issue Jan 12, 2021
karimnaaji added a commit that referenced this issue Jan 13, 2021
- Don't conditionally set uniforms binders, this may lead to potential visual
artifacts during frame with stale data
- Fix order of pattern_attribute, must match the CrossFadedCompositeBinder names
- Add render tests
karimnaaji added a commit that referenced this issue Jan 13, 2021
- Don't conditionally set uniforms binders, this may lead to potential visual
artifacts during frame with stale data
- Fix order of pattern_attribute, must match the CrossFadedCompositeBinder names
- Add render tests
karimnaaji added a commit that referenced this issue Jan 13, 2021
- Don't conditionally set uniforms binders, this may lead to potential visual
artifacts during frame with stale data
- Fix order of pattern_attribute, must match the CrossFadedCompositeBinder names
- Add render tests
@karimnaaji karimnaaji mentioned this issue Jan 13, 2021
7 tasks
@karimnaaji karimnaaji linked a pull request Jan 13, 2021 that will close this issue
7 tasks
karimnaaji added a commit that referenced this issue Jan 22, 2021
karimnaaji added a commit that referenced this issue Jan 22, 2021
karimnaaji added a commit that referenced this issue Jan 23, 2021
@karimnaaji
Copy link
Contributor

karimnaaji commented Jan 23, 2021

Fixed the main regression issue in #10284 . The visual corruption you described in #10033 (comment) is a separate issue, I created a specific ticket for it in #10322.

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