Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

refs #60: first cut of POI rendering #68

Merged
merged 14 commits into from
Feb 21, 2014
Merged

refs #60: first cut of POI rendering #68

merged 14 commits into from
Feb 21, 2014

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Feb 15, 2014

Basic POI rendering with existing style (i.e. only alcohol-shop).

  • add basic marker style to stylesheet
  • pass through marker style to PBF conversion
  • marker/point tile parsing, bucket, buffer, shaders, etc.
  • new sprite hosting for 1x/2x variants
  • sprite load accepts screen scale argument

Also, opacity is supported in the stylesheet much like on WebGL but the current stylesheet doesn't use it.

I am wiiiiide open to review here as this is the most C++ code I've ever written. Outstanding issues/questions:

  • Sprite is hosted on Dropbox for now since there isn't a 2x variant on the old URL. I was able to find a pre-made 2x variant in the WebGL project.
  • Not sure about ramifications of actually changing transform.pixelRatio just yet. This is used in bucket.drawPoints to use the right sprite and seems like a good place to persist it. Why was it always 1.0 before?
  • Sprites aren't drawing at all on OS X yet. Can't get debug variants of shaders to work, either, so it's probably something in the draw call. Sprite image properties are coming through ok.
  • Went with glDrawElements instead of glDrawArrays but not sure if it's necessary. As I understand it, we do this with lines and fill outlines (and in general) in order to avoid duplication of vertices in draw calls. This probably isn't an issue with points.
  • Occasional odd clipping issues.

 * add basic marker style to stylesheet
 * pass through marker style to PBF conversion
 * marker/point tile parsing, bucket, buffer, shaders, etc.
 * new sprite hosting for 1x/2x variants
 * sprite load accepts screen scale argument
@@ -54,7 +55,7 @@ void Map::resize(uint32_t width, uint32_t height, uint32_t fb_width, uint32_t fb
transform.height = height;
transform.fb_width = fb_width;
transform.fb_height = fb_height;
transform.pixelRatio = (double)fb_width / (double)width;
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 sort of intentional. If fb_width is not different from width on a retina screen, there is a bug elsewhere.

@kkaefer
Copy link
Contributor

kkaefer commented Feb 17, 2014

Looks very good! I'll have a look and see why the points aren't rendering on OS X.

@incanus
Copy link
Contributor Author

incanus commented Feb 17, 2014

  • On the pixel ratio stuff, IIRC I still saw 1.0 on retina devices, i.e. fb_width = width. I will double-check this, but this was part of my confusion.

  • Went with glDrawElements instead of glDrawArrays but not sure if it's necessary.

    This is ok? Can you elaborate @kkaefer so I can understand a bit more?

  • Occasional odd clipping issues.

    Did you see this at all @kkaefer?

  • Awesome re: OS X. I will take a look too. I just wanted to get the PR in before going offline for the weekend.

@kkaefer
Copy link
Contributor

kkaefer commented Feb 19, 2014

It looks like there are bogus points at 0/0, I'm currently investigating on why that is the case, because they don't seem to be added to the buffer...

@incanus
Copy link
Contributor Author

incanus commented Feb 19, 2014

Cool re: 504b60b and glDrawArrays. Figured that might be more efficient.

Also, the degenerate shader change makes sense.

On 9710fbe, this removes the ability to colorize the marker imagery, right?

@kkaefer
Copy link
Contributor

kkaefer commented Feb 20, 2014

@incanus this is not yet ready as it still doesn't work on OS X, just on iOS. There seems to be a change in how gl_PointCoord works on OS X.

@incanus
Copy link
Contributor Author

incanus commented Feb 20, 2014

Digging into this, but does this have something to do with differences in GL_POINT_SPRITE_COORD_ORIGIN?

@incanus
Copy link
Contributor Author

incanus commented Feb 20, 2014

@kkaefer on OS X you need to glEnable(GL_POINT_SPRITE); first.

I'm not sure where you were with your debugging but I got past that last blocker.

screen shot 2014-02-20 at 3 55 36 pm

@kkaefer
Copy link
Contributor

kkaefer commented Feb 21, 2014

Yeah, that's what I was looking for!

@kkaefer
Copy link
Contributor

kkaefer commented Feb 21, 2014

I think this is good to go!

kkaefer added a commit that referenced this pull request Feb 21, 2014
refs #60: first cut of POI rendering
@kkaefer kkaefer merged commit 1fb4451 into master Feb 21, 2014
@kkaefer kkaefer deleted the poi-render branch February 21, 2014 11:08
@incanus
Copy link
Contributor Author

incanus commented Feb 21, 2014

Badassery. Nice work @kkaefer and thanks for rounding this out.

acalcutt pushed a commit to acalcutt/mapbox-gl-native that referenced this pull request Apr 30, 2022
* Use DisplayLink to prevent crashes in willResignActive

* Fixed screen freeze issue in the DisplayLink PR when bringing app back to foreground. Includes relevant changes from Mapbox PR (mapbox/mapbox-gl-native-ios#432).

Co-authored-by: Jukka Hietanen <jukka.hietanen@accelbit.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants