[vlc-devel] [vlc-commits] commit: Qt: make ground for proper main interface size management ( Jakob Leben )

[Jakob Leben]

On Sun, Mar 28, 2010 at 11:34 AM, Jean-Baptiste Kempf <jb at videolan.org>wrote:

> On Sat, Mar 27, 2010 at 11:51:37PM +0100, git at videolan.org wrote :
> > vlc | branch: master | Jakob Leben <jleben at videolan.org> | Sat Mar 27
> 22:24:50 2010 +0100| [9d9ffe9fba7946afcf6f20f1de9f6b6a3b644f37] | committer:
> Jakob Leben
> >
> > Qt: make ground for proper main interface size management
>
> I object this commit.
>
> minWidthHolder is a hack.
>

It is the only way to properly ensure minimum width if you don't want to
have adaptGeometry().


your videoRoleWidget is a hack too, it doesn't simplify anything, as you
> can see with showTab() function that needs now a switch! (and force a
> resize to 0,0 in certain cases...
>

It is not a hack. In my opinion it is way more natural than the showTab -
restorePrevious stuff.
Ok, maybe it would _seem_ less hackish if I put something like a
b_videoActive somewhere instead of videoRoleWidget* and tested that in
showTab, or tested some other stuff that proves video active/inactive. But
that's a minor thing. I think the major design is right.

The resize to 0, 0 was a temporary way of implementing "compact mode".
Temporary. And I did suggest another solution right there in the code - the
layout size constraint, but it has it's pluses and it's minuses - read on...



>
> you call showTab() everywhere, that is an inline function with a switch!
>

Alright, I spotted that already. So we'll de-inline it.
...And it's certainly easier maintenance and more error proof to call one
function everywhere (we're not in some performance-problematic region),
instead of having to fold your brain over what you should set the
oldStackCentralWidget to before or after every call to
restoreOldStackCentralWidget() or showTab() just so the things don't get
totally screwed up the _next_ time they will be called. That's ridiculous.


> you don't fix the issues that are still present, namely adaptGeometry()
> and mainBasedVideoSize
>

For adaptGeometry() I offered the solution inside showTab(), by using
QLayout->setSizeConstraint(), which has the problem that it fixes both
height and width. But this is the _only_ way that automatically adjusts and
fixes size of a top window to contents. But ironically, it internally does
just the same thing that adaptGeometry() is supposed to do. Therefore, I'm
not all against adaptGeometry(), I think it is really the only thing we can
do besides living with the mentioned problematic aspect of setSizeContraint.
.Well, we could subclass a QLayout to allow constraint in only one
direction, and install it on main window, but then we have to have also
custom main window... is that easier then adaptGeometry()? Or we could send
a patch to Qt team and wait for next Qt release :)

As for mainBasedVideoSize, please read on... this commit is not exactly
about setting the right sizes everywhere, but making it possible.


> And finally, this breaks when you do simple things like
> bgWidget->resize->playlist->playlist, because you don't see the bgWidget
> anymore, which makes it totally useless to see art when playing music.
>

Could you please describe the problem better, and say in what interface mode
that is?

Moreover, as the commit message says, My commit only _makes ground_ for size
management. It makes it possible by removing some constraints and certainly
doesn't work fundamentally against solution of size problems. But it does
not yet implement all the size management. So anybody is free to just go
ahead and implement it on top of that commit.


> Moreover, I told you that this wasn't ok to commit.
>

Well, please proceed, if you can make it better. I've spent enough time and
researched for all the options we can and cannot use and I have reasons to
be convinced that the basic rationale of my commit is right. I think that in
my search I've exhausted all the possiblities and that this commit works the
right way _towards_ solutions of problems, as far as solutions are possible.
Therefore I offered my solution, which I have been keeping back for very
long, just because you said, you would do it. I hope there is better reasons
for objecting this commit, besides division of labour or "reserving" an
issue for someone to solve. If you have them and can prove it by offering a
better solution which demands fundamentally different code than my commit,
than feel free to revert it.

To sum up, after a lot of research of possibilities, I believe this commit
just does the necessary steps towards proper size management, but does not
yet completely implement it. In my opinion the proper implementation can
stand well on the ground of that commit, instead of demanding a
fundamentally different approach.

Best regards,

Jakob
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100328/2ef1fe91/attachment.html>