[Webkit-unassigned] [Bug 26425] Small Refactoring to Consolidate Common GDI Calls

[bugzilla-daemon at webkit.org]

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





------- Comment #5 from aroben at apple.com  2009-06-16 10:42 PDT -------
(From update of attachment 31325)
> + #ifndef GdiUtilities_h

This should be GDIUtilities_h. We always match the case of the file name.

In general we've tried to resist adding *Utilities files, partly because they
can become a catch-all. So I've been trying to think about how we could remove
the need for such a file here.

> + XFORM makeXFORM(float, float, float, float, float, float);

I think it would be better to add an operator XFORM() to the
TransformationMatrix class instead of adding the makeXFORM function. Then we
can change the existing code to use TransformationMatrix::translate to convert
the rect into a matrix, and convert that to an XFORM. Does that make sense?

> + BITMAPINFO bitmapInfoForSize(const IntSize&, bool invertHeight = false);

Boolean parameters make it hard to read code, and optional boolean parameters
make it really easy to make mistakes when the function signature changes. You
could replace the optional boolean with a required two-member enum, like this:

enum BitmapOrientation { TopDown, BottomUp };
BITMAPINFO bitmapInfoForSize(const IntSize&, BitmapOrientation).

Or you could just require the caller to negate the height themselves.

Another option, which would allow moving this function out of GDIUtilities (and
get us a step closer to removing that file altogether), would be to create a
WebCore::BitmapInfo struct that derives from BITMAPINFO. Something like:

struct BitmapInfo : public BITMAPINFO {
    static BitmapInfo create(const IntSize&);
    static BitmapInfo createBottomUp(const IntSize&);
};

> + void fillWithClearColor(HBITMAP);

I'm not sure what to do with this one.


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