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

Add ability to disable overdraw shaders #5555

Merged
merged 4 commits into from
Jul 20, 2016

Conversation

brunoabinader
Copy link
Member

#5371 and #5534 add a second set of shaders for overdraw mode. We should completely disable this with ifdefs or so when we're building a release build for publication to avoid additional shader compilation overhead. Overdraw mode is a debugging tool and shouldn't be in the SDK.

/cc @brunoabinader

@brunoabinader
Copy link
Member

Disabling overdraw implies in actually disabling debug options in general - 👍 for including https://github.com/mapbox/mapbox-gl-native/blob/master/include/mbgl/map/map.hpp#L171-L174 inside those ifdefs?

@brunoabinader
Copy link
Member

Moved discussion about debug options in release mode to #5699.

@brunoabinader
Copy link
Member

Node render test debug/overdraw fails because we are running the render tests on a Travis node build bot in release mode. @mikemorris any objections on switching this bot to debug mode?

@brunoabinader brunoabinader force-pushed the 5555-disable-overdraw-in-release-mode branch from efbb046 to 6916c91 Compare July 15, 2016 12:44
@mikemorris
Copy link
Contributor

@brunoabinader 👍 to BUILDTYPE=Debug on that bot

@brunoabinader brunoabinader force-pushed the 5555-disable-overdraw-in-release-mode branch 2 times, most recently from a3c90ab to 001f472 Compare July 18, 2016 12:46
@brunoabinader
Copy link
Member

@1ec5 @mikemorris could you please 👀 on the changelog changes before 🚢 ?

@@ -6,6 +6,8 @@ Mapbox welcomes participation and contributions from everyone. Please read [CON

* Added [quadkey](https://msdn.microsoft.com/en-us/library/bb259689.aspx) support and limited WMS support in raster tile URL templates. ([#5628](https://github.com/mapbox/mapbox-gl-native/pull/5628))

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: remove this blank line.

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2016

For iOS and macOS, can you mark the enum values with __attribute__((deprecated("…"))); and point out that they're no-ops in the documentation comment? You can condition the deprecated attribute on the DEBUG build variable.

If you think it's still useful to have these options in iosapp, you can conditionally compile the code that adds and carries out the action. For macosapp, change the menu item's validation code to conditionally return YES for debug and NO otherwise.

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2016

If deprecation and conditional compiling is too much trouble, it'd be fine to simply document the two enum options as being nonfunctional.

@brunoabinader brunoabinader force-pushed the 5555-disable-overdraw-in-release-mode branch 3 times, most recently from 5e1bcc4 to 81852dc Compare July 19, 2016 16:20
@brunoabinader brunoabinader force-pushed the 5555-disable-overdraw-in-release-mode branch from 81852dc to 000539a Compare July 20, 2016 09:49
@brunoabinader brunoabinader merged commit 000539a into master Jul 20, 2016
@brunoabinader brunoabinader deleted the 5555-disable-overdraw-in-release-mode branch July 20, 2016 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants