-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5689/#review8346
-----------------------------------------------------------

Ship it!


Ok, the performance should be acceptable and the patch looks good to me. I am 
fine with commiting but please wait for a go from a bigger plasma dev. ;)


/trunk/KDE/kdelibs/plasma/svg.cpp
<http://svn.reviewboard.kde.org/r/5689/#comment8625>

    A bit shorter but not exact the same:
    "id\\s*=\\s*(['\"])(\\d+)-(\\d+)-([^'\"]+)\\1"
    
    If you know a solution to express [^\1] this would be nice, but I think 
this isn't possible.
    
    Whatever this solution should be enaugh for plasma.


- Manuel


On 2010-10-25 15:29:32, Ingomar Wesp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5689/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 15:29:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Previously, if an SVG contained size hinted elements, they were only used 
> when the display size matched the size hint exactly. This patch tries to 
> relax this condition by searching for the smallest size hinted element that 
> is still bigger than the display size (in order for the element to be chosen, 
> it also has to have the same aspect ratio). If no such element can be found, 
> it falls back to the normal element id as passed.
> 
> In order to speed up the lookup (and because it appears to be impossible to 
> access the DOM of an already loaded SvgRenderer), all size hinted element ids 
> are stored in SharedSvgRenderer at load time.
> 
> I think it would be good to change the QRegExp based id fetching into a 
> proper DOM traversal. Are there any convenience functions in KDELibs that 
> allow easy iterating over all elements (couldn't find any) or do I have to 
> implement that myself based on Qt's DOM classes?
> 
> Please tell me what you think... Have I missed something?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/private/svg_p.h 1189690 
>   /trunk/KDE/kdelibs/plasma/svg.cpp 1189690 
> 
> Diff: http://svn.reviewboard.kde.org/r/5689/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ingomar
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to