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

handle error in GlyphSource#getSimpleGlyphs #3980

Closed
wants to merge 126 commits into from

Conversation

stepankuzmin
Copy link
Contributor

Is this a correct approach to #2711? Should we inherit GlyphSource from Evented to fire errors?

if (glyph) glyphs[glyphID] = new SimpleGlyph(glyph, rect, buffer);
}
if (err) {
this.fire('error', {error: err});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly how I would start 👌 .

The next step is to have the error event be propagated up to Style (from where it'll be propagated up to Map). You can see how we do this on ImageSprite here.

stepankuzmin and others added 27 commits January 21, 2017 14:35
No error or warning is printed for missing patterns.

fix mapbox#4660
We switched to 16-bit coordinates in attributes in a84083b, so there's no longer any reason to require that they are divisible by four.

The integration test updates here are due to:

* Underspecified behavior for overlarge text-halo-width values which was perturbed by this change
* A bug in icon-text-fit logic where the width after extending mod 4 was used, instead of the intrinsic icon width
* Other random and insignificant perturbations
This makes pattern usage more like icons, and will be necessary for data-driven *-pattern properties.
…ox#4688)

Manually bind a_data at position 0 for symbolSDF shader. Binding unused/undefined attributes to position 0 causes symbols to not render in Safari.
* Always return image metrics exclusive of padding
* Work with integer coordinates whenever possible
* Eliminate redundant SpriteAtlasElement members
* Fix asymmetric re-padding in getIconQuad when pixelRatio != 1
* Add explanatory comments
* fire  when redoing placement so collision boxes show up when map.showCollisionBoxes is set to true

* add collision box test

* make render tests consistent
it was only used to find shaders (glsl) files, but it's not really
needed since (1) we are using forward slashes anyway, (2) it's ignored
by brfs plugin

removing it stops browserify from attaching browser compliant
implementation of `path` to the resulting bundle
ChrisLoer and others added 19 commits June 26, 2017 14:24
Credit to @lamuertepeluda which provided some of the basis to this
commit at mapbox#4029 (comment)
Change the "locate the user" example with better defaults for tracking a mobile user.
@jfirebaugh
Copy link
Contributor

Hey @stepankuzmin -- I'm in the process of cleaning out our PR queue and it looks like there hasn't been any activity on this one recently. If you're still interested in coming back to it, please let us know and we'll be happy to help you get it to completion. Until then, I'm going to close it out. Thanks again for your contributions!

@jfirebaugh jfirebaugh closed this Jun 28, 2017
@stepankuzmin
Copy link
Contributor Author

Hi @jfirebaugh! I'm interested in completing this PR. @lucaswoj said that we need to propagate error event up to Style. I've updated the PR in 1e990d6. What should I do next?

@jfirebaugh
Copy link
Contributor

Oh, I missed that, sorry! That should be it. Can you rebase on master to make sure the build passes?

@jfirebaugh jfirebaugh reopened this Jun 29, 2017
@stepankuzmin
Copy link
Contributor Author

Whoops. I'm sorry, but it seems that I made merge instead of rebase. Is there any way to revert it?

@jfirebaugh
Copy link
Contributor

Hmm, looks kind of messy. Want to start fresh with a new PR instead?

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

Successfully merging this pull request may close these issues.