Bug 27779 - WINCE PORT: modified graphics files
Summary: WINCE PORT: modified graphics files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 23154
  Show dependency treegraph
 
Reported: 2009-07-28 13:42 PDT by Yong Li
Modified: 2009-07-30 11:09 PDT (History)
2 users (show)

See Also:


Attachments
the patch (16.73 KB, patch)
2009-07-28 13:45 PDT, Yong Li
no flags Details | Formatted Diff | Diff
remove an unnecessary line (16.47 KB, patch)
2009-07-28 13:51 PDT, Yong Li
no flags Details | Formatted Diff | Diff
remove unnecessary changes (15.30 KB, patch)
2009-07-29 10:06 PDT, Yong Li
no flags Details | Formatted Diff | Diff
add some comments for removed ASSERT (15.41 KB, patch)
2009-07-29 12:30 PDT, Yong Li
staikos: review-
Details | Formatted Diff | Diff
more clean up (14.73 KB, patch)
2009-07-30 07:27 PDT, Yong Li
staikos: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2009-07-28 13:42:32 PDT
patch is comming
Comment 1 Yong Li 2009-07-28 13:45:50 PDT
Created attachment 33668 [details]
the patch
Comment 2 Yong Li 2009-07-28 13:51:53 PDT
Created attachment 33669 [details]
remove an unnecessary line
Comment 3 George Staikos 2009-07-28 18:32:41 PDT
Comment on attachment 33669 [details]
remove an unnecessary line

A lot of the things conditionally removed in this patch look suspicious to me.  Are you sure they're correct?
Comment 4 Yong Li 2009-07-29 06:54:48 PDT
(In reply to comment #3)
> (From update of attachment 33669 [details])
> A lot of the things conditionally removed in this patch look suspicious to me. 
> Are you sure they're correct?

Which things do you mean?

I didn't see many things removed. We don't have PlatformGradient.
Comment 5 George Staikos 2009-07-29 07:18:00 PDT
Is it possible to refactor this into a PlatformGradient class that doesn't do any "platform" calls so we still fit within the framework?
Comment 6 Yong Li 2009-07-29 08:45:15 PDT
(In reply to comment #5)
> Is it possible to refactor this into a PlatformGradient class that doesn't do
> any "platform" calls so we still fit within the framework?

Yes. We can leave a void* for it. just a waste of 4 bytes. I'll clean it up
Comment 7 Yong Li 2009-07-29 10:06:00 PDT
Created attachment 33719 [details]
remove unnecessary changes
Comment 8 Yong Li 2009-07-29 12:30:52 PDT
Created attachment 33731 [details]
add some comments for removed ASSERT
Comment 9 George Staikos 2009-07-29 12:37:10 PDT
Comment on attachment 33731 [details]
add some comments for removed ASSERT

Still more ifdef than I'd like to see but I think it's ok for now.  Unfortunately WinCE doesn't make this easy.
Comment 10 George Staikos 2009-07-29 20:49:45 PDT
Comment on attachment 33731 [details]
add some comments for removed ASSERT

        
> +#if !PLATFORM(WINCE) || PLATFORM(QT)
>          PlatformGraphicsContext* platformContext() const;
> +#endif
> +
> +        const Font& font() const;
> +        void setFont(const Font&);

  I think we have a problem here.  Why are these unconditionally added?

  In retrospect I think we need more cleanup.
Comment 11 Yong Li 2009-07-30 06:41:52 PDT
(In reply to comment #10)
> (From update of attachment 33731 [details])
> 
> > +#if !PLATFORM(WINCE) || PLATFORM(QT)
> >          PlatformGraphicsContext* platformContext() const;
> > +#endif
> > +
> > +        const Font& font() const;
> > +        void setFont(const Font&);
> 
>   I think we have a problem here.  Why are these unconditionally added?
> 
>   In retrospect I think we need more cleanup.

Hm... we don't use them. probably it's a garbage from very old revision. I'll do do more cleanup.
Comment 12 Yong Li 2009-07-30 07:27:11 PDT
Created attachment 33777 [details]
more clean up
Comment 13 George Staikos 2009-07-30 11:09:02 PDT
Committed r46590