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

text-pitch-alignment #5288

Merged
merged 5 commits into from
Jun 11, 2016
Merged

text-pitch-alignment #5288

merged 5 commits into from
Jun 11, 2016

Conversation

yhahn
Copy link
Member

@yhahn yhahn commented Jun 8, 2016

Native port of mapbox/mapbox-gl-js#2668.
native-port

Backlog

cc @lucaswoj @jfirebaugh @mollymerp @lxbarth

@@ -48,7 +48,7 @@ void Painter::renderSDF(SymbolBucket &bucket,
bool alignedWithMap = rotationAlignment == RotationAlignmentType::Map;
float gammaScale = 1.0f;

if (alignedWithMap) {
if (false && alignedWithMap) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll fix this, was poking but this whole codepath can be dropped.

@1ec5 1ec5 added feature navigation For the Mapbox Navigation SDK for Android or iOS or navigation use cases in general Core The cross-platform C++ core, aka mbgl labels Jun 8, 2016
@brunoabinader
Copy link
Member

brunoabinader commented Jun 9, 2016

Good stuff @yhahn! Do you know if this affects the recently added mbgl::ViewportMode::FlippedY?

Easiest way to test is running make run-qt-qml-app

@yhahn yhahn force-pushed the unskew branch 4 times, most recently from 7ec107d to e91dba9 Compare June 10, 2016 17:46
@yhahn yhahn force-pushed the unskew branch 2 times, most recently from 0f13843 to 756d83b Compare June 10, 2016 20:37
@yhahn yhahn changed the title [notready] Unskew line labels on pitched maps text-pitch-alignment Jun 10, 2016
@yhahn
Copy link
Member Author

yhahn commented Jun 10, 2016

@jfirebaugh 🙏 ready for your review when you have a chance : )

@@ -6,12 +6,14 @@ const spec = require('mapbox-gl-style-spec').latest;
var parseCSSColor = require('csscolorparser').parseCSSColor;

global.camelize = function (str) {
str = str === undefined ? 'undefined' : str;
Copy link
Contributor

Choose a reason for hiding this comment

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

What prompted this change, and the corresponding one in camelizeWithLeadingLowercase?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was to have Undefined be the value for text-pitch-alignment when it's not defined in the style and be able to detect when inheritance should kick in (per mapbox/mapbox-gl-style-spec#460 (comment)).

It's definitely the main eyebrow-raiser in this PR 😄 would love to know if there's a better way to achieve this.

Copy link
Contributor

@jfirebaugh jfirebaugh Jun 10, 2016

Choose a reason for hiding this comment

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

Yeah, I see the AlignmentType::Undefined value. Why does a JS undefined value pass through camelize though?

@jfirebaugh
Copy link
Contributor

👌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl feature navigation For the Mapbox Navigation SDK for Android or iOS or navigation use cases in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants