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

Marco Martin notmart at gmail.com
Mon Jul 21 17:39:51 UTC 2014



> On July 21, 2014, 4:26 p.m., Marco Martin wrote:
> > src/plasma/framesvg.cpp, line 480
> > <https://git.reviewboard.kde.org/r/119330/diff/5/?file=291470#file291470line480>
> >
> >     are you sure about this one?
> >     doesn't contentGeometry returns the rectangle adjusted only by frame topHeight/RightHeight etc?
> >     
> >     the semantics of the function is supposed to return the rect adjust by visual margins for layout purposes (so the margin hints elements if present wins against the actual margin graphics, just like marginSize()).
> 
> Aleix Pol Gonzalez wrote:
>     FrameSvgPrivate::contentGeometry takes them into account, because this one does use the values in FrameData.
> 
> Marco Martin wrote:
>     yeah, but I see from FrameData it's using frame->leftWidth, while in this case it should use frame->leftMargin
>     (leftMargin being the same of leftWidth if there is no hint, the size of the hint instead)
> 
> Aleix Pol Gonzalez wrote:
>     Can you create a small test where I can reproduce the issue?
> 
> Marco Martin wrote:
>     I think for at least measures should be possible to do an autotest.. I'll give it a try

Here you go, in master there is an autotest that fails(QEXPECT_FAIL) comparing contentsRect


- Marco


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


On July 21, 2014, 4:40 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, 4:40 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/framesvgitem.h e155f6a 
>   src/declarativeimports/core/framesvgitem.cpp 8320212 
>   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_helpers.h PRE-CREATION 
>   src/plasma/private/framesvg_p.h 8aceef2 
>   tests/dialog.qml PRE-CREATION 
>   tests/testborders.qml PRE-CREATION 
> 
> 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/addf30ef/attachment.html>


More information about the Plasma-devel mailing list