Bug 124999 – [Texmap] TextureMapperLayer unnecessary object copying

• Component: Layout and Rendering
• Status: RESOLVED DUPLICATE

[Przemyslaw Szymanski]

In C++ it is not a good practice to make copy of objects. Mostly because object copying is not cheap operation for CPU especially if an object is large.

[Przemyslaw Szymanski]

Created attachment 218039 [details]
object copying

[Przemyslaw Szymanski]

Comment on attachment 218039 [details]
object copying

wrong attachment format

[Przemyslaw Szymanski]

Created attachment 218041 [details]
patch

[Brent Fulgham]

Comment on attachment 218041 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=218041&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348
>      for (size_t i = 0; i < rects.size(); ++i) {

Please make this a C++11 loop.

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:369
> +        const IntRect& rect = rects[i];

Ditto.

[Przemyslaw Szymanski]

Created attachment 224737 [details]
patch

[Przemyslaw Szymanski]

(In reply to comment #4)
> (From update of attachment 218041 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=218041&action=review
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348
> >      for (size_t i = 0; i < rects.size(); ++i) {
> 
> Please make this a C++11 loop.
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:369
> > +        const IntRect& rect = rects[i];
> 
> Ditto.

Thank you for review. Patch is updated. I hope you thought about range-for not std::foreach

[Brent Fulgham]

Comment on attachment 224737 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224737&action=review

Looks very go! Pleaseo switch to 'auto&', but otherwise this looks good.

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:348
> +    for (IntRect& rect : rects) {

Please make this for (auto& rect : rects) {

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:367
> +    for (IntRect& rect : rects) {

Ditto.

[Fujii Hironori]

r174168 fixed this.

*** This bug has been marked as a duplicate of bug 137271 ***