<div class="gmail_quote">On Sun, Mar 28, 2010 at 11:34 AM, Jean-Baptiste Kempf <span dir="ltr"><<a href="mailto:jb@videolan.org">jb@videolan.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
On Sat, Mar 27, 2010 at 11:51:37PM +0100, <a href="mailto:git@videolan.org">git@videolan.org</a> wrote :<br>
<div class="im">> vlc | branch: master | Jakob Leben <<a href="mailto:jleben@videolan.org">jleben@videolan.org</a>> | Sat Mar 27 22:24:50 2010 +0100| [9d9ffe9fba7946afcf6f20f1de9f6b6a3b644f37] | committer: Jakob Leben<br>

><br>
> Qt: make ground for proper main interface size management<br>
<br>
</div>I object this commit.<br>
<br>
minWidthHolder is a hack.<br></blockquote><div><br>It is the only way to properly ensure minimum width if you don't want to have adaptGeometry().<br><br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

your videoRoleWidget is a hack too, it doesn't simplify anything, as you<br>
can see with showTab() function that needs now a switch! (and force a<br>
resize to 0,0 in certain cases...<br></blockquote><div><br>It is not a hack. In my opinion it is way more natural than the showTab - restorePrevious stuff.<br>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.<br>
<br>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... <br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
you call showTab() everywhere, that is an inline function with a switch!<br>
</blockquote><div><br>Alright, I spotted that already. So we'll de-inline it.<br>...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.<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">you don't fix the issues that are still present, namely adaptGeometry()<br>
and mainBasedVideoSize<br></blockquote><div><br>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 :)<br>
<br>As for mainBasedVideoSize, please read on... this commit is not exactly about setting the right sizes everywhere, but making it possible.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

And finally, this breaks when you do simple things like<br>
bgWidget->resize->playlist->playlist, because you don't see the bgWidget<br>
anymore, which makes it totally useless to see art when playing music.<br></blockquote><div><br>Could you please describe the problem better, and say in what interface mode that is?<br><br>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.<br>
 </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Moreover, I told you that this wasn't ok to commit.<br></blockquote><div> </div><div>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.<br>
<br>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.<br>
<br>Best regards,<br><br>Jakob<br><br></div></div>