Misplacing an embedded window of VLC

[KO Myung-Hun]

Hi/2.

An embedded window of VLC when playing a video is misplaced. The y-coordinate of a window moves to upper than the expected.

And when changing Aspect Ratio and Crop many times, the same problems occur.

Current, VLC uses a workaround for this.

I attach non-workaround version.

[KO Myung-Hun]

Non-workaround version of KVA plugin

[KO Myung-Hun]

You should copy kva_plug.dll to /usr/local/lib/vlc/plugins/video_output.

[Dmitry A. Kuminov]

Hi Ko. Could you please make a hint (if you know) on what part of VLC code does embedding? And what code provides the embedded window?

BTW, VLC 1.1.9 fails here because it cannot open SDDHELP$ (I have Panorama installed). I have to manually change the mode to DIVE to make it work. Is it expected?

[Dmitry A. Kuminov]

I found where it all goes on, the Qt interface is rather simple there. I'm now checking what's going on.

[Dmitry A. Kuminov]

I wrote a simple test case, tests/embedded, that does roughly the same ("externally" draws directly to HWND). But so far, I can't reproduce the problem. I guess it's related to the layout management and my simple test case does something differently compared to the original VLC application.

[KO Myung-Hun]

SDDHELP$ problem was reported by many users.

VLC(libkva) tries to use SNAP overlay, WarpOverlay! and then DIVE on auto mode.

And when checking SNAP overlay, it seems to quit due to SDDHELP$ problem.

These codes causing to quit are in SNAP SDK.

BTW, I have a question, do you have SDDHELP.SYS in CONFIG.SYS ?

Currently, I have no Panorama machine. After installing it, I'll investigate this.

[Dmitry A. Kuminov]

I see. No, I don't have SDDHELP.SYS in CONFIG.SYS. I'm not sure if Panorama supports that.

[KO Myung-Hun]

Ok.

Try this,

​http://www.ecomstation.co.kr/komh/testcase/snapwrap_test.zip

Unzip it into your /vlc2/usr/local/lib.

[Dmitry A. Kuminov]

I have finally reproduced the problem with the embedded test case (see r1018).

VLC basically does this:

    // [1] resize the video widget to match the size of the video
    videoWidget->resize (800, 600);
    // [2] resize the main widget to match the size of the vide widget
    resize(size() - stackCentralW->size() + QSize(800, 600));

According to my understanding of Qt, this is *completely* wrong because videoVidget position and size is managed by a QLayout subclass and therefore must be not moved or sized manually. In this particular case, the line marked with [2] (i.e. resizing the main window) is totally enough -- videoWidget will get its size (800 x 600 as well) automatically thanks to its QLayout. If only [2] is used, the test case doesn't reveal any problems with the wrong position of videoWidget and I believe VLC would not have them as well.

However, for some reason, the Windows version of Qt behaves correctly in this case (and I assume the Linux version as well). This may be just an accident or maybe Nokia deliberately made this case supported because many people did the same mistake... In either case, we have to support this poor behavior on OS/2 as well.

BTW, doing stable->setAutoFillBackground (true); in VideoWidget::request() is a bad idea either. I suppose this is done to paint empty bands on sides of the video frame with black color when the aspect ratios of the video and the window don't match, but this introduces a terrible flicker when e.g. resizing the window during playback. A smarter solution would be to set the window color of VideoWidget itself to black and fix the constraints of the stable widget (using e.g. heightForWidth()) so that it's aspect ratio always matches the video frame size -- this way, the VideoWidget's layout would simply center the stable widget inside it and no flicker would occur since setAutoFillBackground() is not necessary in this case.

[Dmitry A. Kuminov]

Regarding comment:7, I tried the new DLL and now it doesn't terminate when I set the KVA mode to auto. I assume it selects DIVE.

[Dmitry A. Kuminov]

I found where the problem is. Its due to the fact that we return (CVR_ALIGNLEFT | CVR_ALIGNTOP) in response to WM_CALCVALIDRECTS to cause child windows to be aligned to the top left corner of the parent (as this is what Qt expects).

However, in case of the erroneous behavior described above, the flow of order events generated by Qt (as the result of the layout management) is such that the video widget is first aligned by Qt to fit its parent and then the parent is resized which causes PM to re-align the video widget according to the (CVR_ALIGNLEFT | CVR_ALIGNTOP) rule (since it doesn't know it's already aligned). As a result, the video widget gets placed above the bottom edge of the parent to the same amount of pixels it was moved below it when it was bigger (since it was explicitly resized first by VLC).

I can't simply disable this alignment done by PM because we need this in case when the child widgets are not controlled by the parent's layout manager (e.g. when there is no manager at all). OTOH, we can't ask PM to move only some widgets (those which are not under control of the layout manager), it can only align all or none.

Perhaps, I will have to disable this alignment AND align these widgets on my own. I need to think about it.

[Dmitry A. Kuminov]

To be more exact, the problem is that the Qt geometry of the widget is set before its PM geometry (WinSetWindowPos). In particular, when QWidget::setGeometry() is called for a child widget, the order of events is as follows:

QWidget::set_geo(geo)
{
  data->qt_geo = geo;
  map_qt_geo_to_pm_geo();
}

QWidget::map_qt_geo_to_pm_geo()
{
  pm_geo = data->qt_geo;
  // process pm_geo
  WinSetWindowPos(data->hwnd, pm_geo);
  foreach (child, children)
    child->map_qt_geo_to_pm_geo()
}

In other words, if we have one widget (w) that has a parent (p) and one chlid (c), all having size 500x500 px and then call w->set_geo({<old_pos>; 800x600}), this will happen (keep in mind that in PM the Y axis is flipped, so zero is at the bottom):

w->data->qt_geo = 800x600;
    // w->data->pm_geo is still {0,0; 500x500}

WinSetWindowPos(c->data->hwnd, {0, 100; 500x500});
    // w resized vertically by 100 px, but in terms 
    // of Qt, top-left corner of c is still at {0,0} within
    // it counting from top-left, which means that in terms
    // of PM c must be moved 100 px up

WinSetWindowPos(w->data->hwnd, {0,-100, 800x600);
    // 1. w resized vertically by 100 px, but in terms
    // of Qt, its top-left corner is till at {0,0} within
    // p counting from top-left, which means that in terms
    // of PM w must be moved 100 px down
    //
    // 2. PM knows that we want CVR_ALIGNLEFT | CVR_ALIGNTOP for
    // children and, trying to keep the c's top-level corner at
    // the same distance from the w's top-left corner (which is 
    // 100 px above it), it moves c to {0, 200} -- this is totally
    // wrong in turns of Qt because it will mean that the top-left
    // corner of c is at {0; -200} i.e. behind the w's boundaries!

Then the layout manager of w kicks in and it then moves c to its correct position of {0,0, 800x600} both in Qt and PM terms (luckily).

To be continued...

[Dmitry A. Kuminov]

Now what happens when the size of the top-level window (t) containing p (i.e. the parent of w from the last comment) is increased to 800x600 as well:

t->data->qt_geo = {0,0; 800x600}

WinSetWindowPos(c->data->hwnd, {0, 0; 800x600});
    // same as before, nothing changed

WinSetWindowPos(w->data->hwnd, {0, -100; 800x600});
    // same as before, nothing changed

WinSetWindowPos(p->data->hwnd, {0, 100; 500x500});
    // t resized by 100 px, p moved 100 px up in PM
    // to keep left corners aligned

WinSetWindowPos(t->data->hwnd, {0, 0; 800x600});
    // due to CVR_ALIGNLEFT | CVR_ALIGNTOP, p is moved
    // by PM to {0, 200} (see above), so that in terms
    // of Qt it's at {0,-200} now

//// the layout manager of t kicks in here and resizes p

p->data->qt_geo = {0,0; 800x600}
    // p->data_pm_geo is still {0,-200; 500x500}

WinSetWindowPos(c->data->hwnd, {0, 0; 800x600});
    // same as before, nothing changed

WinSetWindowPos(w->data->hwnd, {0, 0; 800x600});
    // since qt_geo of p is now 800x600 too, w is
    // aligned right here both in terms of Qt and PM,
    // however since that PM size of p is still 500x500
    // yet, this moves the top-left corner of w to 
    // {0, -100} in terms of Qt

WinSetWindowPos(p->data->hwnd, {0, 0; 800x600});
    // since qt_geo of t is now 800x600 too, p is
    // aligned right here both in terms of Qt and PM,
    // and since pm_geo of t is also 800x600, p is 
    // completely in bounds of t here. However, when
    // aligning w, PM keeps its top-left corner 100 px
    // above the p's top-left corner by setting its
    // PM geo to {0,200; 800x600}

//// the layout manager of p kicks here but it does
//// nothing with w because it's size is already 800x600
//// and Qt thinks it's properly top-left aligned (since
//// Qt itself didn't move the widget in its own terms)
//// however, it is actually *NOT*. w is at {0,-200} now
//// in terms of Qt, which gives a 200 px gap at the bottom

If we come back to VLC, the gap described in the above paragraph is what we see when the movie window is resized for the first time.

[Dmitry A. Kuminov]

After reading what I wrote above for the second time, I found a simple and nice solution to the problem, r1019: Using the new parent's height (set in Qt but not yet propagated to PM) for flipping the Y coordinate is wrong; the real height in terms of PM must be used.

This fixes the issue in VLC and yet lets us keep returning (CVR_ALIGNLEFT | CVR_ALIGNTOP) in response to WM_CALCVALIDRECTS so PM does top-left alignment for widgets not managed by Qt layout managers.