I think that this is due to the div tag, but I haven't verified this. This may be a regresssion introduced by one of the recent set of checkins for bug 114921, bug 116093 and bug 116109 . I'll attach a test case. To see the correct behaviour try the test case in 0.9.7. In the current builds the plugin is offset down the page. SetWindow is getting the proper values, in this case x = 50, y = 50, width = 200, and height = 200 . I can't debug this further at the moment as my local build crashes on start-up.

Improved test case, this shows the bug better. When the plugin is initially drawn it is in the wrong place. If the window is resized then it jumps to the correct place. Given that the x and y values that are passed to SetWindow never change (they are always 50, 50) this implies that the DC we are initially given is for the plugin area (so we should draw at 0,0 to get the correct plugin position), after the resize we are instead given the DC for the div's layer and so should then draw at (50, 50). I would have thought that the DC should always be for the div's layer, and that worked in 0.9.7, any ideas as to what might have changed to break this?

David, is this still happening?

Peter, I've just tested it in build 2002022003 and the problem still exists. I think we might need someone from layout to help us on this one, can you cc someone suitable. Adding regression keyword as this problem didn't occur in 0.9.7.

adding keywords and I'll see if I can take a look. I'll also ask the layout folks about any changes. Maybe something changed in nsObjectFrame:Paint()?

The nsObjectFrame:Paint()method is the same as in the 0.9.4 branch, so I don't think the problem is there. I think the problem is with the DC that we get from aRenderingContext. It is changing from initally being one that just represents the plugin area, to after the resize representing the whole div tag.

Is anything changing with the views?

I have had a go at debugging this further. What I have tried so far is to find why the device context changes after a resize. I put a breakpoint on: aRenderingContext.RetrieveCurrentNativeGraphicData(&hdc); in nsObjectFrame::Paint and stepped into this method then noted the this pointer. During the plugins first load and paint we get passed rendering contexts with four different addresses (in this order) in my case they were: 0x03dd8750, 0x03c9e9e8, 0x02beccd8, 0x03cf8678, so the final one that we draw to is 0x03cf8678. After the resize the rendering context we are passed is 0x03c9e9e8, which is the second one in the list above. I can't work out where this difference comes from though and why we get passed all those different contexts as we are being loaded. I'll try again when I have some more time.

-->mozilla1.0 Strange behavior with the RenderingContext. Kevin, do you know why it changes after a resize?

Okay, I've got a bit further in understanding this. It's down to Invalidation. During the load process, our plugin is calling InvalidateRect and passing in the whole of its area. So, the first DC we get is the whole of the background and the plugin draws in the right place. The subsequent DCs are just for the invalidated regions, however the plugin still draws at the same offset into the DC. So, it gets drawn in the wrong place. You get equally bad behaviour if you partially invalidate the screen by dragging a window over part of it. After a resize, we get the DC for the whole window again and draw in the right place.

nominating to nsbeta1+ as per ADT triage.

I'm not sure what the correct behaviour should be here. This is something that never happened in 4.x, I guess Mozilla's drawing code is better optimised. There is no way of telling within the plugin what the origin of the DC is. I guess the only way of doing this is using the window.x, and window.y parameters that are passed through SetWindow. This would mean that these parameters could be negative. Here is an example of how things might work if this happened. E.g. the plugin window in the example fills the area of the screen (50, 50, 250, 250): - on the first draw the DC corresponds to the region (0, 0, 250, 250) and the plugin should draw at (50, 50, 250, 250). - if you then invalidate the whole plugin area, then the DC corresponds to (50, 50, 250, 250), how do you tell the plugin this? I would guess that you have to pass (0, 0, 200, 200) through SetWindow. - if you then invalidate just the bottom right area of the plugin say (150, 150, 200, 200) in plugin co-ords. This means that a DC corresponding to (200, 200, 250, 250) will be passed to draw. And (-150, -150, 50, 50) must then be passed through SetWindow. And the plugin must make sure that it only paints into the positive part of the DC. If we adopt this approach the downside is that it will probably break existing windowless 4.x plugins, our plugin as it stands is not designed to deal with negative values for window.x and window.y. Although it's not very much work to fix it. Is there an alternative, can we switch off the optimised drawing for the widget that contains us?

I just found bug 49743 which is for the Mac. It switches double-buffering off for Mac plugins. I guess we could do that for windowless plugins and that might fix this problem.

Does the 0.9.4 branch show this problem? Does disabling double buffering help?

The 0.9.4 branch doesn't show this problem. I haven't tried disabling double-buffering yet. I don't have time today, and am out of office until Wednesday. I'll give it a go then.

David, you are right. There in an optimization in the trunk to create a new context for just the area to be painted. Some of that translation code is here: http://lxr.mozilla.org/mozilla/source/view/src/nsViewManager.cpp#698 Kevin, is there a way to deactivate the optimization? A hack in plugin code may break backwards compatibility, see comment #12.

Looks like we may have to call SetWindow on every paint. The origin will need to be fixed up (with negative values) so that the plugin basically paints at 0,0. Ari, does this bug pose any problems for Viewpoint?

Yes- I see this problem. Haven't been able to play with it though- jury duty :(

The view manager should be applying a translation to the rendering context so that (0,0) is at the origin of the frame whenever you paint. You should never have to apply any sort of fixup. Always start drawing at (0,0). Do not apply the offset you got from SetWindow. If, for backwards compatibility reasons, you must apply your last SetWindow offset when drawing, then we need to modify nsObjectFrame::Paint to subtract that offset before calling into the plugin.

Upon the first paint, the rendering context is the size of the entire window and painting at dirty.x, dirty.y is in the corner. Adding window->x, window->y paints in the right place. +-----------+ -- | | | <-- G | +---+ | | +--------+ | | -- | |--+ | | |------+ +--------+ Upon invalidating the lower left corner of the plugin, the plugin draws in the correct horizontal direction but is offset by G in the veritcal. Is there some kind of translation or other calculation needed to tell the plugin where the right place to paint from the new origin is? Another testcase: http://lxr.mozilla.org/mozilla/source/modules/plugin/tools/sdk/samples/winless/ (add /I "..\..\..\base\public")

Ahhhhhh What's happening is that when the view manager applies a translation to nsRenderingContextWin, the translation is stored in the context's mTranMatrix. But the plugin is using the underlying DC directly, ignoring any accumulated translations. Thus the setup that the view manager is doing for you is just being ignored. You need a way to extract the current translation offset from an nsRenderingContextWin and apply it to the DC before asking the plugin to paint. You'll also need to subtract whatever SetWindow offset the plugin is going to add. (Don't forget to undo your changes before returning.)

Really what you want is a method on nsRenderingContextWin that lets you sync up a DC with whatever logical transformations have been specified for the rendering context. Better talk to whowever owns that code.

This patch improves things but doesn't fix everything. Invalidation works well with this patch. If you drag a window over the top of the plugin then it redraws perfectly. However, sometimes the plugin doesn't appear at all on first load, I need to investigate whether this is related to the patch or not. This is also quite a big change as it knocks out the call to GetWindowOriginInPixels which was introduced to fix a lot of problems viewpoint were having. I suspect though that this might be the 'right' way of doing things.

Looks good! You're definitely on the right track.

The transformation also holds a scale, does anyone know in what circumstances the DC will get scaled? I guess that the scale could be used to modify the height and width passed into SetWindow. Another benefit of the above patch is that it fixes a bug I have been meaning to file for ages. style.mozOpacity never worked with windowless plugins, the plugin always drew in the wrong place. With this patch it works. Also, it's worth noting that I didn't have to change our plugin code at all.

The scale is mainly used for twips->pix conversion. You don't need to worry about it. We don't currently support arbitrary scaling.

Comment on attachment 73005 [details] [diff] [review] First attempt at a patch This patch is pretty good, I'm not seeing any weird problems. Maybe we should try to get this into 0.9.9? r=peterl

I wouldn't bother with this for 0.9.9.

Comment on attachment 73005 [details] [diff] [review] First attempt at a patch sr=roc+moz Looks good for 1.0

Without this patch, windowless plugins are not usable at all. This patch is also isolated to windowless plugins.

Feel free to ask the other drivers for 0.9.9 approval.

Comment on attachment 73005 [details] [diff] [review] First attempt at a patch a=asa (on behalf of drivers) for checkin to the 1.0 trunk

This test doesn't exhibit the problem of sometimes not drawing. I have also included a demo of the mozOpacity working. It's a bit slow in debug builds but it works. The problem of the plugin sometimes not drawing, was due to our plugin. There was a bug in NS4.x which meant that sometimes it set window coords of (0,0) and actually drew the plugin at this position even if it was meant to be offset into the window. So, we ignore calls to draw at (0,0) unless an extra parameter is set in the embed tag. I don't have cvs permission to check-in the patch so could you do it Peter?

Patch in trunk, marking FIXED. David, thanks for the patch! You can get CVS write access by following the guidlines here: http://www.mozilla.org/hacking/getting-cvs-write-access.html

verified with build 2002031803

Nominating for the 0.9.9 branch.

a=chofmann and edt for the 0.9.9 branch.

landed on 0.9.9 branch