Bug 22582

Summary: Need API to generate images from nodes on Windows
Product: WebKit Reporter: Steve Falkenburg <sfalken>
Component: WebKit Misc.Assignee: Steve Falkenburg <sfalken>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
Attachments:
Description Flags
Implement renderedImage for Windows aroben: review+

Description Steve Falkenburg 2008-12-01 17:28:32 PST
To match Mac, we need an API that generates an HBITMAP from a Node.
Comment 1 Steve Falkenburg 2008-12-01 17:30:35 PST
Created attachment 25648 [details]
Implement renderedImage for Windows
Comment 2 Adam Roben (:aroben) 2008-12-01 18:35:55 PST
Comment on attachment 25648 [details]
Implement renderedImage for Windows

> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,18 @@
> +2008-12-01  Steve Falkenburg  <sfalken@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        WARNING: NO TEST CASES ADDED OR CHANGED
> +
> +        * page/Frame.h:
> +        * page/win/FrameCGWin.cpp:
> +        (WebCore::imageFromRect):
> +        (WebCore::imageFromSelection):
> +        (WebCore::Frame::nodeImage):
> +        * page/win/FrameCairoWin.cpp:
> +        (WebCore::imageFromNode):
> +        * page/win/FrameWin.h:

You should reference this bug in your ChangeLog, and add some specific comments about the functions you changed/added.

> +#if PLATFORM(WIN)
> +
> +public:
> +    HBITMAP nodeImage(Node*) const;

Someday it would be nice for nodeImage to return a WebCore::Image and for the implementations to be shared. Maybe you could add a FIXME?

> +HBITMAP Frame::nodeImage(Node* node) const
> +{
> +    RenderObject* renderer = node->renderer();
> +    if (!renderer)
> +        return 0;
> +
> +    IntRect topLevelRect;
> +    IntRect paintingRect = renderer->paintingRootRect(topLevelRect);
> +
> +    d->m_view->setNodeToDraw(node); // invoke special sub-tree drawing mode
> +    HBITMAP result = imageFromRect(this, paintingRect);
> +    d->m_view->setNodeToDraw(0);
> +
> +    return result;
>  }

I think we should call updateLayout() before calling imageFromRect, to match the FrameMac implementation. Maybe imageFromRect should be the one to call updateLayout?

> +++ WebCore/page/win/FrameWin.h	(working copy)
> @@ -34,6 +34,7 @@ typedef struct HBITMAP__* HBITMAP;
>  namespace WebCore {
>  
>      HBITMAP imageFromSelection(Frame* frame, bool forceWhiteText);
> +    HBITMAP imageFromNode(Frame*, Node*);

I don't think you need this declaration.

> +++ WebKit/win/ChangeLog	(working copy)
> @@ -1,3 +1,12 @@
> +2008-12-01  Steve Falkenburg  <sfalken@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * DOMCoreClasses.cpp:
> +        (DOMElement::renderedImage):
> +        * DOMCoreClasses.h:
> +        * Interfaces/DOMPrivate.idl:

Same comments about the ChangeLog here as in WebCore.

> +HRESULT STDMETHODCALLTYPE DOMElement::renderedImage(HBITMAP* image)
> +{
> +    if (!image) {
> +        ASSERT_NOT_REACHED();
> +        return E_POINTER;
> +    }
> +    *image = 0;
> +
> +    ASSERT(m_element);
> +
> +    WebCore::Frame* frame = m_element->document()->frame();

We shouldn't be using the WebCore:: prefix in .cpp files.

r=me
Comment 3 Steve Falkenburg 2008-12-01 21:27:58 PST
Fixed in r38894.