Preview - Add label index overlay text to preview screen #38
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sur5r
reviewed
Nov 16, 2018
glabels/Preview.cpp
Outdated
| model::Distance minWH = min( w, h ); | ||
|
|
||
| char labelText[32]; | ||
| snprintf(labelText, sizeof(labelText), "%d", labelInstance); |
There was a problem hiding this comment.
Wouldn't this be better implemented using QString? E.g.:
auto labelText = QString::number(labelInstance);
There was a problem hiding this comment.
@sur5r yep, thats much simpler, corrected that and removed the rotation support and re-pushed for re-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First draft of adding label index overlay text to the preview screen. The snprintf() seemed like it could be out of place but I didn't see any other value to string conversion going on. Also it didn't seem great to assume the order of the origins corresponding to the index of the label, unless of course that is a safe assumption.
I also didn't see any rotation support in the preview screen so maybe the rotation flag and code can/should be removed if it isn't necessary.