Skip to content

Conversation

bertini97
Copy link
Contributor

Hi. Based on the fact that #69 was discarded in favour of #70 because of the non-antialiasing, I made very slight changes to make the lines align better to the pixels and not have 2px transparent borders. I found this trick here.

Here are some results (with red border in the light scene to enhance the comparison). Left is unpatched.
border

Sorry If I'm this pedantic, but Qt 6.8 introduced this as default on GNOME, and the transparent border is an eye sore, and really noticeable when the window is tiled left or right, because you can then see your background.

Cheers

@bertini97 bertini97 changed the title pixel perfect border Border alignment for antialiasing Nov 12, 2024
@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

Please, squash all the commits into one once you are ready and fix clang-format check. I will try to look into it as soon as possible. Also, what about pushing this to Qt upstream? It looks even the previous change I did (#70) is not there. Maybe that will fix your issue? Or is that change not enough.

@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

I tested this patch on top of QtWayland and it seems to work really well after initial testing. I just had to fix the border at the bottom (see code suggestion).

@bertini97
Copy link
Contributor Author

I'm sorry, squashing the commits deleted your code suggestion 😅. I'll resquash after. Sorry again

@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

I'm sorry, squashing the commits deleted your code suggestion 😅. I'll resquash after. Sorry again

No, I actually forgot to submit it. Can you please also update your commit message? Using final commit will be weird in the history. Also a describing what this change is about in the commit message also helps.

@bertini97
Copy link
Contributor Author

Done. Sorry for the naming, I assumed the commits woudn't appear in the final tree.

@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

There is still one formatting issue there with the one I suggested. Can you please fix that? Does my suggestion work for you? It has fixed the border at the bottom for me.

@bertini97
Copy link
Contributor Author

There is still one formatting issue there with the one I suggested. Can you please fix that? Does my suggestion work for you? It has fixed the border at the bottom for me.

Yes, it works. Thank you. Should be properly formatted now.

@bertini97
Copy link
Contributor Author

Also, what about pushing this to Qt upstream? It looks even the previous change I did (#70) is not there. Maybe that will fix your issue? Or is that change not enough.

I thought they were pulling from this repo directly. In any case, it's enough if it lands in qadwaitadecorations, the next version of Qt6 hopefully picks it up.

@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

Also, what about pushing this to Qt upstream? It looks even the previous change I did (#70) is not there. Maybe that will fix your issue? Or is that change not enough.

I thought they were pulling from this repo directly. In any case, it's enough if it lands in qadwaitadecorations, the next version of Qt6 hopefully picks it up.

No, I implemented the decoration in QtWayland myself. I can backport the other change and this one if you allow me to, of course mentioning you as the author.

@grulja grulja merged commit 506c1fe into FedoraQt:main Nov 13, 2024
3 checks passed
@bertini97
Copy link
Contributor Author

No, I implemented the decoration in QtWayland myself. I can backport the other change and this one if you allow me to, of course mentioning you as the author.

Feel free, don't worry about crediting. However, please consider a unification, because Qt's upstream decorations share the same name as these, but are missing shadows which are a major usability improvement. Users might be confused as to why they have the "adwaita decorations" but missing some functionality.

@bertini97 bertini97 deleted the pixelperfect branch November 13, 2024 13:24
@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

No, I implemented the decoration in QtWayland myself. I can backport the other change and this one if you allow me to, of course mentioning you as the author.

Feel free, don't worry about crediting. However, please consider a unification, because Qt's upstream decorations share the same name as these, but are missing shadows which are a major usability improvement. Users might be confused as to why they have the "adwaita decorations" but missing some functionality.

I might backport shadows there too, even though the implementation here is not perfect.

@grulja
Copy link
Contributor

grulja commented Nov 13, 2024

grulja added a commit that referenced this pull request Nov 19, 2024
grulja added a commit that referenced this pull request Nov 20, 2024
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