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

Improve map debug options #3100

Merged
merged 6 commits into from
Nov 27, 2015
Merged

Improve map debug options #3100

merged 6 commits into from
Nov 27, 2015

Conversation

brunoabinader
Copy link
Member

Besides tile identification and status, I believe it would be a nice feat to have the associated tile timestamp information.

By timestamp I refer to the time point used together with expiry date to calculate if the cached tile data is still valid.

/cc @mapbox/gl

@brunoabinader brunoabinader self-assigned this Nov 25, 2015
@brunoabinader
Copy link
Member Author

Example output:
screenshot from 2015-11-25 16-06-34

Please beware that you'll need to clear your current SQLite cache file, as these changes modifies the type for both modified and expires variables.

kkaefer
kkaefer reviewed Nov 25, 2015
View reviewed changes
src/mbgl/renderer/debug_bucket.cpp
const std::string text = std::string(id) + " - " + TileData::StateToString(state);
fontBuffer.addText(text.c_str(), 50, 200, 5);

if (modified > 0 && expires > 0) {
const std::string modifiedText = "modified: " + util::rfc1123(modified);
Copy link
Member

Choose a reason for hiding this comment

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

Can we find a shorter representation of the date, e.g. "Y-m-d H:i:s" and imply GMT?

@tmpsantos
Copy link
Contributor

Neat 👍

@friedbunny
Copy link
Contributor

Cool, this should be useful.

@incanus
Copy link
Contributor

incanus commented Nov 25, 2015

As on GL JS, we should probably look into splitting this out separately from tile borders/labels and collision boxes. Things are getting crowded and turning one on means turning all on, which sometimes gets in the way.

@tmpsantos
Copy link
Contributor

As on GL JS, we should probably look into splitting this out separately from tile borders/labels and collision boxes. Things are getting crowded and turning one on means turning all on, which sometimes gets in the way.

Maybe we should have something like ::enableDebug(DebugOptions)

@brunoabinader brunoabinader force-pushed the 3100-tile-debug-timestamp branch 6 times, most recently from 3395d5a to e37218e Compare November 26, 2015 21:02
@brunoabinader
Copy link
Member Author

Maybe we should have something like ::enableDebug(DebugOptions)

@incanus @tmpsantos good idea. I've added MapDebugMode enum in the last patch to specialize the debug information shown on each tile. Map::toggleDebug() now increases the amount of debug information on screen until it goes back to none. I've kept current behavior i.e. MapDebugMode::TileBorders | MapDebugMode::ParseStatus when debug is set for each port, but ports can now specialize this if they want/need.

The example below better illustrates the new behavior for Map::toggleDebug():
test

@tmpsantos
Copy link
Contributor

I've added MapDebugMode enum in the last patch to specialize the debug information shown on each tile. Map::toggleDebug() now increases the amount of debug information on screen until it goes back to none.

❤️

@brunoabinader brunoabinader force-pushed the 3100-tile-debug-timestamp branch from e37218e to 3acfa72 Compare November 27, 2015 08:49
@brunoabinader brunoabinader changed the title Tile timestamp information in debug mode Improve map debug modes Nov 27, 2015
@brunoabinader
Copy link
Member Author

Text collision debug is now a flag MapDebugOptions::Collision. I've noticed there is currently a bug in collision debug implementation that causes it to not properly repaint upon toggling - this will be handled in #3140.

@brunoabinader brunoabinader force-pushed the 3100-tile-debug-timestamp branch from 3acfa72 to 8bf2215 Compare November 27, 2015 09:17
kkaefer
kkaefer reviewed Nov 27, 2015
View reviewed changes
src/mbgl/map/map_data.hpp
}

inline bool getCollisionDebug() const {
return collisionDebug;
inline void toggleDebug() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename toggleDebug to reflect what it actually does? It's not really toggling rather than cycling through the options.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point @kkaefer 👍 In that sense MapDebugMode should be also called MapDebugOptions instead. I'll update the code accordingly.

@brunoabinader brunoabinader changed the title Improve map debug modes Improve map debug options Nov 27, 2015
@brunoabinader brunoabinader force-pushed the 3100-tile-debug-timestamp branch 6 times, most recently from f2bb1de to 7b034f4 Compare November 27, 2015 15:31
@brunoabinader
[core] Replace time_t with std::chrono::seconds
31fd282 
Added aliases for std::chrono typedefs (eg. 'Seconds' for
std::chrono::seconds). These aliases are used together with templated
helper functions to replace time_t with std::chrono::seconds for most
cases, in particular for 'modified' and 'expires' values in Response.
@brunoabinader brunoabinader force-pushed the 3100-tile-debug-timestamp branch from 7b034f4 to 92856f3 Compare November 27, 2015 15:47
@brunoabinader brunoabinader merged commit 92856f3 into master Nov 27, 2015
@brunoabinader brunoabinader deleted the 3100-tile-debug-timestamp branch November 28, 2015 13:05
@friedbunny friedbunny mentioned this pull request Nov 30, 2015
Closed
@1ec5 1ec5 mentioned this pull request Jan 29, 2016
Merged
1ec5 added a commit that referenced this pull request Jan 29, 2016
@1ec5
[ios] Restored toggleDebug
6995452 
Restored (but deprecated) a method removed in #3100 to avoid a major version bump.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@brunoabinader brunoabinader

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@brunoabinader @tmpsantos @friedbunny @incanus @kkaefer @jfirebaugh @mourner