[Webkit-unassigned] [Bug 63502] [EFL] Add EflWidgetBackingStoreCairo

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 19 18:17:48 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=63502





--- Comment #11 from EunMi Lee <eunmi15.lee at samsung.com>  2011-09-19 18:17:47 PST ---
(In reply to comment #8)
> (From update of attachment 98845 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98845&action=review
> 

Thank you for your comments.

> > Source/WebCore/platform/efl/EflWidgetBackingStoreCairo.cpp:51
> > +        /* get cairo_surface from widget */
> 
> Comments in webkit begin with a capital and end with a period (full sentences).  I don't hintk this comment is helpign undrestand what's going on.
> 
> > Source/WebCore/platform/efl/EflWidgetBackingStoreCairo.cpp:54
> > +        m_surface = adoptRef(cairo_image_surface_create_for_data
> > +                             ((unsigned char*)pixels, CAIRO_FORMAT_ARGB32, w, h, w * 4));
> 
> c++ cast in a c++ file please.
> 
> > Source/WebCore/platform/efl/EflWidgetBackingStoreCairo.cpp:57
> > +        /* create scroll surface */
> 
> Not helpful.  Also, you'd use a // comment anyway.
> 

I modified all comments in the code as your advice,
and type-casting code is removed but I will be careful to use type-casting.

> > Source/WebCore/platform/efl/EflWidgetBackingStoreCairo.cpp:86
> > +    targetRect.shiftMaxXEdgeTo(targetRect.maxX() - scrollOffset.width());
> > +    targetRect.shiftMaxYEdgeTo(targetRect.maxY() - scrollOffset.height());
> 
> I'm surprised there isn't a combined function which takes an IntSize?  Is this just the same as calling setSize on the rect?
> 

Yes, actually it is same with setSize with checking that size is bigger than 0.
So, I modified above two line as follows:
+    targetRect.setWidth(std::max(0, targetRect.width() - scrollOffset.width()));
+    targetRect.setHeight(std::max(0, targetRect.height() - scrollOffset.height()));

> > Source/WebCore/platform/efl/EflWidgetBackingStoreCairo.cpp:93
> > +    copyRectFromOneSurfaceToAnother(m_private->m_surface.get(), m_private->m_scrollSurface.get(),
> > +                                    scrollOffset, targetRect);
> > +    copyRectFromOneSurfaceToAnother(m_private->m_scrollSurface.get(), m_private->m_surface.get(),
> > +                                    IntSize(), targetRect);
> 
> I'm not sure wrapping is helping readability here.

That codes are for coping reusable area within the m_surface when scrolling.
m_surface's reusable area is copied to m_scrollSurface (temporary),
and then copied area of m_scrollSurface is copied to the m_surface again.
For that, I use the copyRectFromOneSurfaceToAnother function in the CairoUtilities.cpp.
Do you think it is better to use cairo operation directly?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list