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

Add a parameter to the text feature to limit what gets rendered. #834

Merged
merged 2 commits into from
Jun 29, 2018

Conversation

manthey
Copy link
Contributor

@manthey manthey commented May 31, 2018

If specified, this skips rendering text that is outside of the current viewport by a specified distance. If you have very large text or text with offsets, this could fail to render the text. However, in well-formed cases, this can skip rendering text that would not be visible in any case.

@manthey manthey force-pushed the text-feature-rendered-zone branch from e66138e to e8151a4 Compare June 1, 2018 19:17
@@ -60,6 +60,12 @@ var feature = require('./feature');
* stroke color. May include opacity.
* @property {geo.geoColor|function} [style.textStrokeWidth=0] Text stroke
* width in pixels.
* @property {number|function} [style.renderedZone] If this is a positive
Copy link
Member

Choose a reason for hiding this comment

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

@manthey i am not entire sure if the name of the variable truly reflects the intention. Would you be open to call it something like renderThreshold or viewportThreshold or something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of those, I think renderThreshold would be clearer, or even a longer combination like renderViewportThreshold. Do you have a preference?

Copy link
Member

Choose a reason for hiding this comment

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

renderThreshold seems good enough for me now. Since we do not use the word viewport otherwise, I would have chosen renderViewportThreshold.

the code LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to renderThreshold and rebased to current master.

@manthey manthey force-pushed the text-feature-rendered-zone branch from d58e155 to ade9fef Compare June 27, 2018 19:59
manthey added 2 commits June 27, 2018 16:01
If specified, this skips rendering text that is outside of the current
viewport by a specified distance.  If you have very large text or text
with offsets, this could fail to render the text.  However, in
well-formed cases, this can skip rendering text that would not be
visible in any case.
@manthey manthey force-pushed the text-feature-rendered-zone branch from ade9fef to 95f10c9 Compare June 27, 2018 20:01
@manthey manthey merged commit e8b5062 into master Jun 29, 2018
@manthey manthey deleted the text-feature-rendered-zone branch June 29, 2018 19:26
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.

2 participants