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

Map debug options in release mode #5699

Closed
brunoabinader opened this issue Jul 15, 2016 · 6 comments
Closed

Map debug options in release mode #5699

brunoabinader opened this issue Jul 15, 2016 · 6 comments
Labels
Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Node.js node-mapbox-gl-native

Comments

@brunoabinader
Copy link
Member

We use map debug options mainly as a mechanism to inspect our rendering engine. This usually comes with additional overheads that potentially affects metrics like binary size and memory footprint - see #5555 as example. In light of this, I see two possibilities in which we could avoid exposing this in release mode:

  • Make all MapDebugOptions no-ops in release mode - this way we'd still have functions like Map::setDebug available in release mode to avoid any binary incompatibility, but their internal implementation would provoke no effects in the engine.
  • Do not expose MapDebugOptions, Map::setDebug and related public API functions in release mode - this adds extra complexity for platform-specific code that would also need to remove their related functions in release mode.

I'm inclined to accept the former option due to its low complexity and maintenance - however I'd love to read your thoughts before proceeding with this.

@mapbox/gl

@brunoabinader brunoabinader added the Core The cross-platform C++ core, aka mbgl label Jul 15, 2016
@kkaefer
Copy link
Member

kkaefer commented Jul 15, 2016

no-ops in release mode

We could log a message to warn the user that this doesn't do anything to avoid confusion.

@1ec5
Copy link
Contributor

1ec5 commented Jul 15, 2016

These are public APIs in the iOS and macOS SDKs, documented as aids for debugging styles. We'll need to update the documentation to note that they're no longer functional, are deprecated, and exist only for backwards compatibility.

@brunoabinader
Copy link
Member Author

Per conversation with @1ec5 - it turns out that the tile boundary and tile info debug flags are currently being very useful for bug reporting. It seems reasonable then to make MapDebugOptions::Overdraw and MapDebugOptions::StencilClip no-ops and remove those from the public SDKs.

@1ec5
Copy link
Contributor

1ec5 commented Jul 15, 2016

They're already public, and I don't think it's worth breaking backwards compatibility over these flags, so we'd mark them as deprecated and document them as nonfunctional for now.

@mikemorris
Copy link
Contributor

These are all currently exposed in the Node.js bindings as well, and we've used them a little for debugging, but they're not clearly documented anywhere AFAIK.

@mikemorris mikemorris added Node.js node-mapbox-gl-native macOS Mapbox Maps SDK for macOS iOS Mapbox Maps SDK for iOS labels Jul 15, 2016
@brunoabinader
Copy link
Member Author

I believe we have general consensus to make both overdraw and stencil clip debug options no-ops in release mode. This is now being proposed in #5555.

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 iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS Node.js node-mapbox-gl-native
Projects
None yet
Development

No branches or pull requests

5 participants