Bug 22790 - [Transforms] MediaPlayer::setRect() makes no sense with transforms
Summary: [Transforms] MediaPlayer::setRect() makes no sense with transforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords:
Depends on:
Blocks: 15670
  Show dependency treegraph
 
Reported: 2008-12-10 10:56 PST by Simon Fraser (smfr)
Modified: 2009-03-06 11:26 PST (History)
2 users (show)

See Also:


Attachments
proposed patch (13.89 KB, patch)
2009-03-06 10:28 PST, Eric Carlson
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2008-12-10 10:56:14 PST
MediaPlayer has a setRect() method. However, this implies that the video is not rotated, so it makes no sense if there is a transform on the video, or an ancestor.
Comment 1 Eric Carlson 2009-03-06 10:28:14 PST
Created attachment 28366 [details]
proposed patch
Comment 2 Simon Fraser (smfr) 2009-03-06 10:37:03 PST
Comment on attachment 28366 [details]
proposed patch

> Index: WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp
> ===================================================================

> -    //TODO: m_rect vs rect?
> +    //TODO: m_size vs rect?

Is the TODO still applicable?

> Index: WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h
> ===================================================================

> +    IntRect m_rect;

I have a minor preference for just storing a size in the member var,
and constructing a (0,0,size) rect where you need one.

> Index: WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm

> +    , m_rect(IntRect())

No need to initialize. IntRect has a ctor.

> +    	if (m_player->inMediaDocument() && r != m_rect) {
> +			// the QTMovieView needs to be placed in the proper location for document mode
> +        	m_rect = r;
> +			[view setFrame:m_rect];

Tabs here?

r=me with those comments addressed.
Comment 3 Eric Carlson 2009-03-06 10:52:12 PST
>> Index: WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp
>> ===================================================================
>>
>> -    //TODO: m_rect vs rect?
>> +    //TODO: m_size vs rect?
>
> Is the TODO still applicable?

I don't know if it is still applicable so I left the comment.

>> Index: WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h
>> ===================================================================
>>
>> +    IntRect m_rect;
>
>I have a minor preference for just storing a size in the member var,
>and constructing a (0,0,size) rect where you need one.

It make sense to store the Rect passed to  paint() when we are in a media
document so I think I will leave the member var as a rect
Comment 4 Eric Carlson 2009-03-06 11:10:12 PST
http://trac.webkit.org/changeset/41486