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

Implement Widget Rendering Sandboxing #14

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

simotek
Copy link

@simotek simotek commented Apr 28, 2023

Sorry for the delay, had some life / work stuff going on and there was
a couple of bugs and I wanted to try and find a way to better separate
the drawing of background and foreground but cairo isn't very flexible
in that regard.

This uses cairo_surface_create_for_rectangle to give each widget
it's own cairo_t, this means that Widgets can no longer draw
outside the current boundry that is set for them.

Unfortunately this also means the backgrounds need to also be redrawn
each cycle to make semi transparent shapes render correctly. With the way
conky work's I don't know how that actually worked before.

cairo_surface_create_for_rectangle was added to conky in v1.19.1

This uses cairo_surface_create_for_rectangle to give each widget
it's own cairo_t, this means that Widgets can no longer draw
outside the current boundry that is set for them.

cairo_surface_create_for_rectangle was added to conky in v1.19.1
* Now paint the backgrounds in the pre section.
* Unfortunatly we need to clear and repaint all backgrounds every
  cycle, unless we want to start tracking which widgets overlap
  others.
@philer
Copy link
Owner

philer commented May 6, 2023

Hey, thanks for your contribution!

I'm wondering what the benefit of this change is? I understand that it limits widgets from drawing outside their box but it seems to me like that would just hide a bug in that particular widget. I'd rather fix widgets that do this unintentionally and not inhibit widgets that might do this on purpose.

Having to redraw backgrounds on each update may also have a relevant performance impact, so I'm not sure if it's worth the trade off. Back when I separated background rendering I got a noticeable performance improvement.

@simotek
Copy link
Author

simotek commented May 8, 2023

The main culprit I saw here was cairo's text implementation which I will eventually move away from anyway but basically without this we'd have to check the length of every generated string and trim it if the result was too long.

I was also partly thinking for longer term sustainability if multiple people started submitting widgets then we wouldn't need to worry as much about them doing wrong things in corner cases.

But after reading your comment today I went back and had another play this afternoon and managed to get the background drawing to another surface today so it doesn't need to be recreated each cycle. But its getting late and currently its in a branch with split widgets so i'll rebase it to this branch and submit it tomorrow.

Rather then re rendering widget backgrounds every update they
are now rendered to a separate surface which is remerged with the
main surface when it is cleared.
@simotek
Copy link
Author

simotek commented May 9, 2023

Updated now, in theory this version should be faster although the old version performed just fine on my machines.

Another reason for taking this approach is with the old version if any widget code did a transform and wasn't reset correctly it would affect all the other widgets which may not be easy to track, where as this implementation limits a widgets transform to there own drawing.

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