Review Request 119330: Convert FrameSvgItem to use 9 tiles instead of a big texture.

Marco Martin notmart at gmail.com
Mon Jul 21 15:25:55 UTC 2014



> On July 21, 2014, 2:09 p.m., Marco Martin wrote:
> > src/plasma/framesvg.h, line 290
> > <https://git.reviewboard.kde.org/r/119330/diff/4/?file=291452#file291452line290>
> >
> >     err, it's not really how I intended in the previous review.. adding public api is even worse...
> >     
> >     those 3 really don't give any extra information compared to what you can obtain with just the plain Svg api.
> 
> Aleix Pol Gonzalez wrote:
>     Well, then we'll have the exact same function in FrameSvgItem and FrameSvg. I'm unsure this is any better.
> 
> David Edmundson wrote:
>     If it's worse we can use the previous version of this patch.

yes, i know that having the same function in the two places is not particularly good.
But between all the options I would really prefer it.
I'm not excluding that a public api for the topology of the pieces may even be a good idea in the future, not sure.
In the meantime, if they are well separed, even if means a bit of duplication of code, leaves more room for future modifications rather than a piece going public


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119330/#review62788
-----------------------------------------------------------


On July 21, 2014, 1:51 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119330/
> -----------------------------------------------------------
> 
> (Updated July 21, 2014, 1:51 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Use FrameSVG as 9 tiles instead of uploading a big texture of the finished frame each time.
> 
> This also saves the cache being populated with full created frames in different sizes; which end up taking up space in the disk and shared memory cache as well as the GPU memory.
> 
> A code path falls back to the original uploading the entire texture if obscure settings are used, i.e overlay.
> 
> Benchmarks:
>  - apitrace when resizing a frame goes from an average of 7.6ms per frame of *CPU* time just for the swizzling and uploading to 1.4ms
>   
>  - GPU time also drops from 40us to 10us
> 
> Themes will need to remove stretch-borders (when we gain nothing from stretching; i.e Breeze) to get the most out of it. 
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/svgitem.cpp 1ed0631 
>   src/declarativeimports/core/tooltipdialog.cpp e62ed6e 
>   src/plasma/framesvg.h dd6d8da 
>   src/plasma/framesvg.cpp fcc6809 
>   src/plasma/private/framesvg_p.h 8aceef2 
>   tests/dialog.qml PRE-CREATION 
>   tests/testborders.qml PRE-CREATION 
>   src/declarativeimports/core/framesvgitem.cpp 8320212 
>   src/declarativeimports/core/framesvgitem.h e155f6a 
> 
> Diff: https://git.reviewboard.kde.org/r/119330/diff/
> 
> 
> Testing
> -------
> 
> Tested oxygen + breeze + some random (and ugly) themes from kde-look.
> 
> Theme changes work.
> 
> Everything looks the same; including the borders on oxygen.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140721/adea0792/attachment-0001.html>


More information about the Plasma-devel mailing list