Skip to content
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

atlas,d2d: overdraw the background bitmap by one cell on all sides #17674

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Aug 6, 2024

BackendD2D will now draw one extra cell on all sides when rendering the background, filled with the expected background color, starting at (-1, -1) to ensure that cell backgrounds do not bleed over the edges of the viewport where the is swapchain but no content.

Fixes #17672

BackendD2D will now draw one extra cell on all sides when rendering the
background, filled with the expected background color, starting at (-1, -1)
to ensure that cell backgrounds do not bleed over the edges of the
viewport.

Fixes #17672
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Aug 6, 2024
@DHowett DHowett requested a review from lhecker August 6, 2024 18:05
The translation in `transform` ensures that we render it off the top left of the render target.
*/
auto backgroundFill = std::make_unique_for_overwrite<u32[]>(static_cast<size_t>(size.width * size.height));
std::fill_n(backgroundFill.get(), size.width * size.height, u32ColorPremultiply(p.s->misc->backgroundColor));
Copy link
Member Author

Choose a reason for hiding this comment

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

A heap alloc for 1440 bytes? At least it only happens when we need to recreate the bitmap.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it some more, you could use CreateCompatibleRenderTarget and Clear() to initialize the bitmap. There's some code you could copy in _resizeCursorBitmap.

Another option is that we preallocate the foreground/background bitmap with a 1px margin and adjust the "stride" accordingly so that D3D is unaffected by this. The latter option is potentially the optimal solution, I think. (I'd be happy to help with that if you'd like.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Arguably more expensive than a heap alloc of 1440 bytes, FWIW.

A compatible render target, pointed to the bitmap, based on the current render target, all to just clear it (after converting the background color to D2D's preferred D2D_COLOR_F) and suck the bitmap out to stride-copy the actual background colors in.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, preallocating it down in the Renderer is a good idea. Can we book that for the future, or would you like it in this PR?

@DHowett
Copy link
Member Author

DHowett commented Aug 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DHowett DHowett enabled auto-merge (squash) August 6, 2024 20:40
@DHowett DHowett merged commit 9007fc2 into main Aug 6, 2024
18 of 20 checks passed
@DHowett DHowett deleted the dev/duhowett/d2d-bleed-no-more branch August 6, 2024 21:50
DHowett added a commit that referenced this pull request Aug 13, 2024
BackendD2D will now draw one extra cell on all sides when rendering the
background, filled with the expected background color, starting at (-1,
-1) to ensure that cell backgrounds do not bleed over the edges of the
viewport where the is swapchain but no content.

Fixes #17672

(cherry picked from commit 9007fc2)
Service-Card-Id: 93494324
Service-Version: 1.21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
Development

Successfully merging this pull request may close these issues.

Background color can exceed the height of the last line
3 participants