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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Sixel images in conhost #17421
Add support for Sixel images in conhost #17421
Changes from 7 commits
6eee84a
dda425a
4b5d784
8795ba3
4acc50d
004a183
7916799
518cc05
e52eef1
673bef9
4853123
8c7587d
91b79c0
6c072bb
dc099c6
bf5c6fe
baba91b
da3f95d
adf507f
48d203d
6401446
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The for loop advances the iterator by
_cellSize.height * _pixelWidth
in total which is equal to the_pixelBuffer
size. If theeraseOffset
is greater than 0, the final loop iteration will leave the iterator past the end of the_pixelBuffer
. Since the MSVC STL uses checked iterators this results in a debug assertion.One way to fix this:
FYI
fill_n
isn't super duper optimal for clearing bytes in this loop and you may be better off usingmemset
directly. Mostly because it skips an unnecessary emptiness check. Something like this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bringing this up. It never occurred to me that it would be a problem using
std::advance
passed the end of the buffer, but it makes perfect sense in retrospect. I think I only starting using it because I was getting complaints from the auditor about pointer arithmetic in some places, so I'll be happy to go back to+=
. If there are still pointer arithmetic warnings I'll find another way to deal with that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out
+=
isn't any better thanstd::advance
since it still has the debug asserts. But I found that switching to raw pointers - and continuing to use thestd::advance
for incrementing - was enough to keep everyone happy. Debug build doesn't appear to assert anymore, and audit still passes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I'm using an
RGBQUAD
here just because it was convenient for the GDI renderer, and I figured the Atlas renderer might not care what format it's given. But if this turns out to be a problem, it shouldn't be that big a deal to change to something more generic.