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

Imagebox: add proper svg resizing #3802

Closed
wants to merge 3 commits into from

Conversation

Crylia
Copy link
Contributor

@Crylia Crylia commented Apr 28, 2023

This is part one of a small fix for imagebox. This PR is so imagebox correctly resized SVG's, previously it was just rendered by cairo (which worked fine) but for the svg to be properly recolored and rendered the Rsvg object needs to know about its future size, thats why I had to switch to render_document (its what the docs recommend to use anyway) which now takes an extra RsvgRect argument which is a simple stuct with its w/h and x/y(not needed) information

I also added seperate logic to scale the cairo surface. This was needed because the original scale would also automatically scale the SVG again, instead It now scales the rect, and the temporary surface (I needed a new one with the new size, else the original wouldn't fit the resized SVG anymore)

@Crylia
Copy link
Contributor Author

Crylia commented Apr 28, 2023

facepalm I'll fix the formatting thing (thanks github addon for randomly opening the PR lol)

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #3802 (22d1560) into master (b6263bf) will increase coverage by 0.02%.
Report is 24 commits behind head on master.
The diff coverage is 100.00%.

❗ Current head 22d1560 differs from pull request most recent head df6bc1d. Consider uploading reports for the commit df6bc1d to get more accurate results

@@            Coverage Diff             @@
##           master    #3802      +/-   ##
==========================================
+ Coverage   90.99%   91.02%   +0.02%     
==========================================
  Files         900      901       +1     
  Lines       57502    57770     +268     
==========================================
+ Hits        52325    52586     +261     
- Misses       5177     5184       +7     
Flag Coverage Δ
gcov 91.02% <100.00%> (+0.02%) ⬆️
luacov 93.71% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/wibox/widget/imagebox.lua 96.52% <100.00%> (+0.98%) ⬆️

... and 9 files with indirect coverage changes

cr:scale(aspects.w, aspects.h)
if self._private.handle then
RsvgRect = Rsvg.Rectangle {x = 0, y = 0, width = w*aspects.w, height = h*aspects.h }
svg_surf = cairo.ImageSurface(cairo.Format.ARGB32, w*aspects.w, h*aspects.h)
Copy link
Member

Choose a reason for hiding this comment

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

the stacktrace from the failed test actually pointing to this line, but anyway you got the idea :)

Copy link
Member

Choose a reason for hiding this comment

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

you marked thread as resolved but test is still failing on this line: https://github.com/awesomeWM/awesome/actions/runs/6042238743/job/16477140356?pr=3802

Elv13
Elv13 previously approved these changes Aug 30, 2023
@Elv13
Copy link
Member

Elv13 commented Aug 30, 2023

@Crylia apology for the long delay. I wasn't paying attention much to all PRs lately. Can you rebase this so the CI can run?

@actionless actionless closed this Aug 31, 2023
@actionless
Copy link
Member

@Elv13 i've re-opened it to re-run - it should merge without manual rebase

@actionless actionless reopened this Aug 31, 2023
cr:scale(aspects.w, aspects.h)
if self._private.handle then
RsvgRect = Rsvg.Rectangle {x = 0, y = 0, width = w*aspects.w, height = h*aspects.h }
svg_surf = cairo.ImageSurface(cairo.Format.ARGB32, w*aspects.w, h*aspects.h)
Copy link
Member

Choose a reason for hiding this comment

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

you marked thread as resolved but test is still failing on this line: https://github.com/awesomeWM/awesome/actions/runs/6042238743/job/16477140356?pr=3802

@actionless
Copy link
Member

also aside from failing test, code-style check is also not passing: https://github.com/awesomeWM/awesome/actions/runs/6042238743/job/16477141387?pr=3802

@Elv13
Copy link
Member

Elv13 commented Dec 31, 2023

Moved to #3881 because I don't have write access to this PR

@Elv13 Elv13 closed this Dec 31, 2023
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.

4 participants