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

Clarify constants and calculations in SDF rendering #4229

Open
anandthakker opened this issue Feb 8, 2017 · 2 comments
Open

Clarify constants and calculations in SDF rendering #4229

anandthakker opened this issue Feb 8, 2017 · 2 comments

Comments

@anandthakker
Copy link
Contributor

There are several constants and calculations in draw_symbol.js and the symbol_sdf shaders that are difficult to interpret / understand. I'm pretty much with @lucaswoj when he says:

I prefer a very clear constant name (ideally googleable) to a magic number, and a magic number to a cryptic constant name.

(though I'd say a semi-clear constant name with a one-line comment to explain it still beats a magic number in my book)

@anandthakker
Copy link
Contributor Author

anandthakker commented Feb 8, 2017

cc @lbud @ansis based on my best guess from reading through some of the git history in these files.

Examples of inscrutable constant names:

  • SDF_PX (was sdfPx)
  • gammaScale => u_gamma_scale (compare w/ v_gamma_scale :scream`)
  • EDGE_GAMMA (was just u_gamma I think)
  • blurOffset (now inlined as 6.0 in the frag shader)
  • buff (was u_buffer) (btw, is it a coincidence that 6/8 (present in calculation for the halo version of this value) = 0.75 = (265 - 64)/256 (the non-halo version of this value)?)

@anandthakker
Copy link
Contributor Author

Thanks to a tip from @ChrisLoer, noticed that TinySDF appears to have some default constant values that line up with these:

    this.fontSize = fontSize || 24;
    this.buffer = buffer === undefined ? 3 : buffer;
    this.cutoff = cutoff || 0.25;
    this.fontFamily = fontFamily || 'sans-serif';
    this.radius = radius || 8;

https://github.com/mapbox/tiny-sdf/blob/master/index.js#L8-L12

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

No branches or pull requests

1 participant