798607 - Rendering layout problems for videos with screen resolution > 100% on win7

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

Using the latest hourly build, cset: 
https://hg.mozilla.org/mozilla-central/rev/3b458f4e0f42 

STR:
1.  load www.youtube.com
2.  Play any video 
3.  Not smallist/badly rendered display, scrolling destroys the video.

win7 x64 running cset above m-c hourly build win32 

Also using the latest beta build of Flash, but there are no problems with 
cset: https://hg.mozilla.org/mozilla-central/rev/fd724f194a1f Today's Nightly

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Vidoes on www.cnn.com are also affected, display is shrunken to top left corner, and scrolling chops the video into multiple layers.

Comment 2

[Jim Jeffery not reading bug-mail 1/2/11]

Regress range:

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ac4d3cec62a1&tochange=676ffbbc4e03

With the uplift coming early next does this make it a blocker ?

I don't/can't do my own builds so can't backout locally.

guessing bug 603880 ?

Comment 3

[Jim Jeffery not reading bug-mail 1/2/11]

Attachment: Render problem w/scroll

Note the small window in upper left of viewing area and its overlayed on what was background.  Scrolling only adds multiples of the image.

Comment 4

[Loic]

Another example: http://boingboing.net/2012/10/05/entire-working-mobile-phone-w.html
Snapshot: http://i.imgur.com/NY2p5.jpg (embedded videons in the upper left corner of the video area)

Comment 5

[Alice0775 White]

Attachment: test

I can only reproduce when I change system Screen Resolution.
And it is only windowed plugin.

Steps to reproduce:
1. Open Screen Resolution by clicking the Start button
2. Clicking Control Panel, and then, under Appearance and Personalization, clicking Adjust screen resolution.
3. Choose Medium - 125%
4. Click Apply. And logout / login

5. Start Firefox and Open URLs


Regression window:
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ac4d3cec62a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121004193627
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/676ffbbc4e03
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121004194128
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ac4d3cec62a1&tochange=676ffbbc4e03

In local build:
Last good: a4c95cde82e9
First bad: 6c0312e3fe59
Triggered by:
6c0312e3fe59	Robert O'Callahan — Bug 603880. Implement nsWindow::GetDefaultScale on Windows based on LOGPIXELSY, set by font DPI settings. r=jmathies

Comment 6

[Loic]

True, I'm running in 125% resolution right now.

Comment 7

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Loic from comment #6)
> True, I'm running in 125% resolution right now.

Yes, I do to... 
@Alice, thanks for the verification and finding its related to screen resolution settings

Comment 8

[Gary [:streetwolf52]]

Same issue under Windows 8 running 120 dpi.

Comment 9

[u52928]

Win 7 Pro x64 & 64-bit Fx. 

Resolution 111%. 

Same problem. YouTube movie shows in ca. 30% of window -> in top left corner. 

Full screen work fine.

Flash (latest version 11.5.500.85) works fine on another site I tested (e.g. http://edu-net.net/media/talkin2me/).

Immediately before trying today's Nightly, I had updated my graphics adapter driver. (nVidia 306.79-quadro-tesla-win8-win7-winvista-64bit-international-whql.exe) I thought it might be at fault. 

However, I've gone back to Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121005030609 CSet: fd724f194a1f and YouTube plays fine.

Comment 10

[Loic]

Videos on CNN are affected too.

Comment 11

[Loic]

I found a reduced testcase from a previous bug, see attachment https://bugzilla.mozilla.org/attachment.cgi?id=665184

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm confused, this shouldn't have changed anything by default.

In about:config, what is layout.css.devPixelsPerPx? It should be 1.0...

Comment 13

[gabe]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12)
> I'm confused, this shouldn't have changed anything by default.
> 
> In about:config, what is layout.css.devPixelsPerPx? It should be 1.0...

it reads 1.0 here but still have rendering problem

Comment 14

[Loic]

1.0 here too.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This is a regression from bug 785667 (which is only visible on a platform that implements GetDefaultScale). Platforms other than Mac always use device pixels for plugin coordinates so should always have a GetContentsScaleFactor of 1.0.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The Mac code is also kinda wrong. We should get this ratio directly from the device context rather than trying to predict what it should be.

Comment 17

[Loic]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #15)
> Created attachment 669089 [details] [diff] [review]
> fix
> 
> This is a regression from bug 785667 (which is only visible on a platform
> that implements GetDefaultScale).

So why is the regression visible on Windows only since yesterday?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> The Mac code is also kinda wrong. We should get this ratio directly from the
> device context rather than trying to predict what it should be.

Filed bug 799078 on that, with patches.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Loic from comment #17)
> So why is the regression visible on Windows only since yesterday?

Because until yesterday Windows didn't implement GetDefaultScale :-).

Comment 21

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 669089 [details] [diff] [review]
fix

This is fine for now.

But I think you'll find that plugin coordinates need to be in "display pixels" on all platforms.

Or at the very least, you'll need to ensure that window sizes and coordinates sent to the plugin are in "display pixels", and that these don't change when you zoom the page containing the plugin.

I'll examine these issues more closely in my review of the patch for bug 799078.

Comment 22

[Steven Michaud [:smichaud] (Retired)]

> until yesterday Windows didn't implement GetDefaultScale :-).

I assume you're talking about the following bugs:

https://bugzilla.mozilla.org/show_bug.cgi?id=797828
https://bugzilla.mozilla.org/show_bug.cgi?id=603880

Comment 23

[Jonathan Kew [:jfkthame]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12)
> I'm confused, this shouldn't have changed anything by default.
> 
> In about:config, what is layout.css.devPixelsPerPx? It should be 1.0...

Just FTR, this is not accurate any more: in bug 674373, we changed the default to "-1.0" so that HiDPI and non-HiDPI windows on OS X can both do the right thing.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Jonathan Kew (:jfkthame) from comment #23)
> Just FTR, this is not accurate any more: in bug 674373, we changed the
> default to "-1.0" so that HiDPI and non-HiDPI windows on OS X can both do
> the right thing.

I changed it back, on Windows only, in bug 603880, since Windows isn't ready for "HiDPI" to be enabled by default yet.

(In reply to Steven Michaud from comment #21)
> But I think you'll find that plugin coordinates need to be in "display
> pixels" on all platforms.
> 
> Or at the very least, you'll need to ensure that window sizes and
> coordinates sent to the plugin are in "display pixels", and that these don't
> change when you zoom the page containing the plugin.

On Windows display pixels are always device pixels, as far as I can tell.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/afc8967a5cea

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 669089 [details] [diff] [review]
fix

Review of attachment 669089 [details] [diff] [review]:
-----------------------------------------------------------------

Gruesome Aurora regression, trivial fix.

Comment 27

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #24)

>> But I think you'll find that plugin coordinates need to be in
>> "display pixels" on all platforms.
>>
>> Or at the very least, you'll need to ensure that window sizes and
>> coordinates sent to the plugin are in "display pixels", and that
>> these don't > change when you zoom the page containing the plugin.
>
> On Windows display pixels are always device pixels, as far as I can
> tell.

So when NPP_SetWindow() is called on Windows, NPWindow.x, NPWindow.y,
NPWindow.width and NPWindow.height are in device pixels?  And these
values change when the page containing the plugin is full-zoomed
(making the plugin itself change size)?

I'm looking around for a Flash "movie" that displays (at least) what
mouse events it receives, and what their coordinates are (i.e. which
has the basic functionality of my Debug Events Plugin for the Mac at
bug 441880).  Please tell me if such a beast exists that you know of,
and where it can be found.

Comment 28

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #27)

I've found more or less what I wanted:
http://www.developphp.com/view.php?tid=82

I notice that the coordinates of (say) the upper-left corner of the grid don't change if you zoom the page, on the Mac or on Windows.  This is (I think) the correct behavior.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/afc8967a5cea

Should this have a test?

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Steven Michaud from comment #27)
> So when NPP_SetWindow() is called on Windows, NPWindow.x, NPWindow.y,
> NPWindow.width and NPWindow.height are in device pixels?  And these
> values change when the page containing the plugin is full-zoomed
> (making the plugin itself change size)?

Yes. We've always done it that way, on all platforms including Mac actually!

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Steven Michaud from comment #28)
> I've found more or less what I wanted:
> http://www.developphp.com/view.php?tid=82

It seems to me that this example scales its content to fit the plugin window and translates coordinates accordingly. I get the same results even if I modify width/height attributes on the <embed> to resize the plugin instance.

Comment 32

[Steven Michaud [:smichaud] (Retired)]

> on all platforms including Mac actually!

I don't think so.  But it'll be easy enough for me to check.  I'll do that tomorrow.

Comment 33

[Steven Michaud [:smichaud] (Retired)]

I'll also use the tryserver build that checks on Windows and Linux.

Comment 34

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #30)

>> (In reply to Steven Michaud from comment #27)
>> So when NPP_SetWindow() is called on Windows, NPWindow.x,
>> NPWindow.y, NPWindow.width and NPWindow.height are in device
>> pixels?  And these values change when the page containing the
>> plugin is full-zoomed (making the plugin itself change size)?
>
> Yes. We've always done it that way, on all platforms including Mac
> actually!

Turns out you're right (and I was wrong).

I've done a debugging patch that shows this on the Mac and on Windows.
(I'll post the patch and a tryserver build once I've got it working
properly.)

But we can't just use device pixels with plugins on the Mac (this is
demonstrated by the way plugins behaved after bug 674373 landed but
before bug 785667 landed).  And I still suspect we'll have problems
using device pixels with plugins on other platforms.

During the gap between bug 674373 and bug 785667, some plugins (Java
and my Debug Events Plugin from bug 441880) displayed at the wrong
size (too small).  And all plugins failed to respond properly to mouse
events (even though, as it happens, we were already passing the
correct coordinates in those events).  I think this was due to
confusion in the plugins about how large their windows were on the
screen.  Using device pixels made these windows seem twice as large
(in HiDPI mode) as they actually were.

Comment 35

[Steven Michaud [:smichaud] (Retired)]

Attachment: Debugging patch that logs calls to NPP_SetWindow()

Here's my debugging patch.

And here are the tryserver builds made from it:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-74d96ea9f7cd/

To see its debug logging on Windows, you'll need to use Sysinternal's DebugView (http://technet.microsoft.com/en-us/sysinternals/bb896647.aspx).

Comment 36

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Steven Michaud from comment #34)
> During the gap between bug 674373 and bug 785667, some plugins (Java
> and my Debug Events Plugin from bug 441880) displayed at the wrong
> size (too small).  And all plugins failed to respond properly to mouse
> events (even though, as it happens, we were already passing the
> correct coordinates in those events).  I think this was due to
> confusion in the plugins about how large their windows were on the
> screen.  Using device pixels made these windows seem twice as large
> (in HiDPI mode) as they actually were.

Presumably that's because the plugins use Cocoa APIs that took coordinates in Cocoa units, which are different from device pixels in HiDPI mode, and assumed those coordinates matched the pixel values we passed in.

As far as I know, plugins on other platforms won't have that issue because the platform APIs don't use coordinate systems that differ from device pixels.

Comment 37

[Steven Michaud [:smichaud] (Retired)]

That's certainly possible.  Time (and testing) will tell.

Comment 39

[Jim Jeffery not reading bug-mail 1/2/11]

Did this land in Aurora ?  I've seen a couple of people on 18.0a2 saying the issue still exists.. and a quick scan through m-a, I didn't find it...

Comment 40

[Kevin Brosnan [Ex-Mozilla]]

This has not landed on Aurora yet.

Comment 41

[mehrdad58]

I am using Aurora 18.0a2 with the latest updates (2012-10-15) and I have problem with youtube  videos which are messed up. I noticed now that generally embedded contents are shown incorrectly such as PDF contents. Anyway, I had reported this problem in a different bug report which was then marked as duplicate of this one!

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7574f94957b7

Comment 43

[u52928]

Mozilla/5.0 (Windows NT 5.1; rv:18.0) Gecko/18.0 Firefox/18.0 ID:20121016042009 CSet: 03b50c47e744

Maybe I'm getting ahead of things, but this has been listed as fixed although the bug is still present in today's Aurora.

In case there's any question, at least from a user's standpoint, in YouTube, CNN, der Spiegel, etc., it's exactly the same problem we had last week in the 64-bit Nightly. That is, the video only appears in the top left 1/3 of the window.

In cases of "primitive" embedding of Flash videos - e.g. on my website mentioned in comment 9 - there's no problem.

Comment 44

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Changeset 03b50c47e744 is before 7574f94957b7 so I think your Aurora build doesn't have my patch in it. Try the next Aurora nightly.

Comment 45

[Paul Silaghi, QA [:pauly]]

Saw the issue on Nightly 2012-10-08. 
Everything looks ok on youtube.com, cnn.com, testcase in comment 5, testcase in comment 11 on FF 18b2 on Win 7 x64. Verified fixed.

Comment 46

[Virgil Dicu [:virgil] [QA]]

(In reply to Paul Silaghi [QA] from comment #45)
> Saw the issue on Nightly 2012-10-08. 
> Everything looks ok on youtube.com, cnn.com, testcase in comment 5, testcase
> in comment 11 on FF 18b2 on Win 7 x64. Verified fixed.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 

Looks ok in F19 beta 3 with the above-mentioned sites and testcase

Comment 47

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old