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

AtlasEngine: Implement sixels #17581

Merged
merged 9 commits into from
Jul 23, 2024
Merged

AtlasEngine: Implement sixels #17581

merged 9 commits into from
Jul 23, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 18, 2024

  • Add a revision to ImageSlice so that the renderers
    can use it to cache them as bitmaps across frames.
  • Hooked up the revision tracking to AtlasEngine to cache the
    slices into Buffers so we can own them into the Present.
  • Hooked up those snapshots to BackendD3D with a straightforward
    hashmap -> atlas-rect logic. Just like rendering text.
  • Hooked up BackendD2D with a bad, but simple & direct drawing logic.
  • Bonus: Modify ImageSlice to be returned as a raw pointers
    as this helps performance slightly. (Trivial type == good.)
  • Bonus: Fixed the _debugShowDirty code (disabled by default).

Validation Steps Performed

  • mpv --really-quiet --vo=sixel foo.mp4 looks good ✅
  • Scroll up down & observe dirty rects ✅

@lhecker lhecker added Product-Terminal The new Windows Terminal. Area-AtlasEngine labels Jul 18, 2024
@mominshaikhdevs
Copy link

after merging this PR, maybe close the relevant issue #448 too?

@zadjii-msft
Copy link
Member

after merging this PR, maybe close the relevant issue #448 too?

We'll probably need to merge #17510 before we can close that out

@DHowett
Copy link
Member

DHowett commented Jul 18, 2024

Still failing audit mode! You may want to build it locally ;)

@j4james
Copy link
Collaborator

j4james commented Jul 19, 2024

Two small issues I've noticed which I'm assuming are related to the order in which various layers are rendered:

  1. If you've got text with an underline attribute set, and you draw a sixel image over that, the underlines show through the image. That should not be happening.
  2. If you set the cursor position on top of an image, you can't see the cursor, assumedly because it's rendering underneath the image. It should ideally be visible.

I'm not sure either of them are blockers, but would be good to fix at some point.

Edit: The more I think about, the more I suspect that the cursor issue may not be solvable, at least not for all cursor styles.

@lhecker
Copy link
Member Author

lhecker commented Jul 19, 2024

The more I think about, the more I suspect that the cursor issue may not be solvable, at least not for all cursor styles.

Generally speaking, it is solvable.
But you're effectively right, because it requires rewriting the vertex/pixel-shader approach in BackendD3D with a compute-shader using a tile deferred shading approach. I think I'm finally at a point where I'd know how to build something like that, but I'm not sure if I can still afford working on rendering code.

{
const auto& b = row->bitmap;

// TODO: This could use some caching logic like BackendD3D.
Copy link
Member

Choose a reason for hiding this comment

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

yea I was curious why this wasn't just exactly the same

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot the exact reason, but something about building the hashmap was annoying so I decided to do that in a follow-up PR. The D2D renderer isn't particularly fast anyway, so I suspect the additional overhead here won't make it that much worse either (for now).

@DHowett DHowett merged commit 75f7ae4 into main Jul 23, 2024
19 of 25 checks passed
@DHowett DHowett deleted the dev/lhecker/atlas-engine-sixel branch July 23, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants