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

Fix mirrored displays when transformed and preserve aspect ratio #5697

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

VirtCode
Copy link
Contributor

Describe your PR, what does it fix/add?

Currently, the mirroring of monitors is unusable with different aspect ratios or straight out broken when using transformed monitors.

This PR does the following:

  • When drawing on the mirror framebuffer, it undoes the transforms of the mirrored monitor, so the mirror framebuffer always contains the monitor in the normal transform.
  • When drawing on the mirrored monitor, instead of stretching the framebuffer accross the entire surface, it respects the aspect ratio, meaning there will be black bars if the aspect ratios do not match.

This makes mirroring usable when the source and/or target monitor is transformed. I also think preserving the aspect ratio should be the normal behaviour, as stretching is not what's ideal in most usecases.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

As mentioned, we currently copy the buffer of the monitor into a separate mirror buffer which is then drawn on the target monitors. The buffer of the monitor is already transformed with the monitor transform, so this PR renders the buffer with the inverse transform.

To do that, the transform and the matrices of the monitor are temporarily changed for this one render, which is a bit of a janky solution. Otherwise we'd have to override or add a second transform in the renderTextureInternalWithDamage which would probably require a lot of changes.

Also, for what reason is the monitor framebuffer copied to the mirror buffer, which is then rendered? Couldn't we just directly render the offloadFB of the mirrored monitor on the target?

Is it ready for merging, or does it need work?

The current approach works fine on both my systems, and I did quite a bit of testing with all possible transforms. So I think it is ready.

I could also do a "cleaner" implementation as mentioned above (i.e. pass an extra transform to the render method) if you want.

@vaxerski
Copy link
Member

Also, for what reason is the monitor framebuffer copied to the mirror buffer, which is then rendered? Couldn't we just directly render the offloadFB of the mirrored monitor on the target?

historical reasons for back when it didn't exist.

If you want to, you can make this MR use it (and remove mirrorFB)

vaxerski
vaxerski previously approved these changes Apr 22, 2024
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

lgtm, I assume it works (I am not weird, I dont rotate my monitors)

@vaxerski
Copy link
Member

uh oh, clang-format missing

@VirtCode
Copy link
Contributor Author

VirtCode commented Apr 22, 2024

The transforms and the mirroring are working correctly, but I am seeing some graphical glitches with the blur of floating windows when mirroring from one orientation to another. They seem to be present on the offloadFB but I don't fully know where they come from. I'll try to get a screenshot.

uh oh, clang-format missing

oh yes, my bad, i'll fix it

@VirtCode
Copy link
Contributor Author

VirtCode commented Apr 22, 2024

That's how the artifacts look like. They seem to stem from the damage regions and are only visible on surface which use real (not xray) blur. What's odd is that they only appear when the two monitors have different orientation.
image

I'll have a proper look at it tomorrow.

@vaxerski
Copy link
Member

right thats probably why we used mirroredFB, I forgot. Expanding damage

@vaxerski vaxerski force-pushed the main branch 3 times, most recently from 9b00957 to c3ec16f Compare April 23, 2024 00:49
@VirtCode
Copy link
Contributor Author

After too many hours of experimenting and debugging yesterday, I've also come to the conclusion that we need an extra framebuffer.

The point is that we can't use the offloadFB because of the double buffering of the monitors. A damage area which has only been updated on one buffer, might get overwritten in the next frame, so the second buffer will contain whatever artifacts were written there (as it requires updating in the last and second-to-last damage region).

I guess we could "fix" this by not using double buffering for mirror monitors, or only using the "latest" damage and not the union (i.e. not passing the damage through the monitor's ring). However, that wouldn't stop artifacts from appearing in other scenarios, like when using different refresh-rate monitors, and would be really janky.

So unless you have a better idea, I think using an extra framebuffer is the more robust and reliable solution.

I've now reimplemented the monitorMirrorFB but made a few changes:

  • The mirror FB is now again left untransformed, and only transformed on rendering. This makes the transforming logic easier and more robust. This also easily allows mirroring of things like the cursor zoom.
  • Mirror monitors are now (again?) properly damaged, so this should increase the performance a tiny bit when mirroring.
  • I've also set mirror monitors to never offload, as that doesn't make sense.

I've tested this implementation a fair bit, so I think it should be ready now.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

// yes, this breaks mirrors of mirrors

this shouldn't be allowed anyways lole

lgtm!

@vaxerski vaxerski merged commit 9fe4098 into hyprwm:main Apr 24, 2024
10 checks passed
gulafaran pushed a commit to gulafaran/Hyprland that referenced this pull request Apr 26, 2024
…ratio (hyprwm#5697)

* renderer: transform mirror buffer and preserve mirror aspect ratio

* renderer: render mirrors directly from offloadFB

* renderer: fix formatting

* renderer: use monitorMirrorFB again, but properly damage mirrors

* renderer: clean mirrors after reload and support cursor zoom mirroring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants