Peter Kümmel wrote:
Abdelrazak Younes wrote:
     view.show();
-    view.init();
So you don't need LyXView::init() anymore or is it done elsewhere?

before the if (width ...
I think it makes more sense to initialize before showing and resizing.

Of course yes. I haven't seen it.


// FIXME: some code below needs moving Index: src/frontends/qt3/QtView.C
===================================================================
--- src/frontends/qt3/QtView.C    (revision 14157)
+++ src/frontends/qt3/QtView.C    (working copy)

@@ -55,14 +55,12 @@
-QtView::QtView(unsigned int width, unsigned int height)
+QtView::QtView()
        : QMainWindow(), LyXView(), commandbuffer_(0), frontend_(*this)
 {
-    resize(width, height);
-
     qApp->setMainWidget(this);
- bufferview_.reset(new BufferView(this, width, height));
+    bufferview_.reset(new BufferView(this, width(), height()));
I think there might be a problem there. BufferView needs the size of the
WorkArea but width() and height() will give you the size of the window,
including menubar and toolbars.

No, this is given by frameGeometry, see
http://doc.trolltech.com/4.1/geometry.html
same for Qt3

Hum, frameGeometry doesn't include the "Window Title" but it still include menubar and toolbar IIUC so I guess I am still right :-)


But I guess this is OK now because a resize event will happen just
afterward and readjust the BufferView to the proper WorkArea size. So I
guess that passing 100x100 or whatever will produce the same effect.

menubar_.reset(new QLMenubar(this, menubackend));
     getToolbars().init();
@@ -157,17 +155,62 @@
     return qApp->activeWindow() == this;
 }
+void QtView::initNormalGeometry(const QRect & g)
+void QtView::resetGeometry(const QRect & g)


The functionality of most code here is to have a valid not-maximized geometry:
the normal geometry.
initNormalGeometry does not change the geometry of the window, it's part
of the workaround for the missing normal/maximized functionality on x11/qt3,
so I think 'normal' is still a good choice for the naming.

Hum, we are talking about floating window geometry right? Why do you think about floatingGeometry_ instead of normalGeometry and setFloatingGeometry() instead of initNormalGeometry() ?


+{
+    normalGeometry_ = g;
+    maxWidth=QApplication::desktop()->width()-20;
+}
+QRect QtView::qtViewGeometry() const
+QRect QtView::getGeometry() const


Yes, that's better.

or even better :

+QRect QtView::geometry() const


+void QtView::moveEvent(QMoveEvent *)
+{
+    if (width() < maxWidth && frameGeometry().x() > 0)
+        normalGeometry_ = qtViewGeometry();
This is just for updating x and y position isn't it? Why do we need that
by the way? Because you want to save this in the session info as well. I
guess it make sense on Windows but won't this clash with Window manager
that do this under X11?


Here I update the normal size and position which should be saved when
lyx is closed in a maximized status. normalGeometry_ must not be overwritten
by values of the maximized window, therefore the if.

I didn't questioned the if but the necessity of the moveEvent() method, sorry for the confusion. I mean, if the objective is to save the geometry on exit do that on the closeEvent. Or am I missing something here?



X11/qt3 does not provide the normal size of a maximized windows, because of
this all the code here.

OK.


+#ifndef Q_OS_WIN32
+        // X11: use frameGeometry position
+        view.setGeometry(0, 0, width, height);
+        view.move(posx, posy);
+#else
         view.setGeometry(posx, posy, width, height);
+#endif
         if (maximize)
             view.setWindowState(Qt::WindowMaximized);
I don't like the #ifndef at all. Why do we need them? Why only for qt4?


X11/qt4 is broken, we only need the workaround on X11. Most ifdefs could
be removed, but then we have some useless additional calls on windows, but this
isn't a big problem.

If it isn't harmful remove them please. Or, as I said, encapsulate them in some helper function.


+#ifndef Q_OS_WIN32
+       ///
+       virtual void resizeEvent(QResizeEvent * e);
+
+       ///
+       virtual void moveEvent(QMoveEvent * e);
+#endif
Same here. IMO if these methods are really necessary define them in any
case. The #ifdef code in GuiView.C should then be encapsulated in some
helper functions somewhere that will set the geometry of QWidget
depending on the platform. Please understand that I've worked hard to
avoid special cases in general code and I don't want them to come back.
X11/Mac/WIn specific code should go in helper functions.


There's no problem removing the ifdefs. But I would prefer to have the
ifdef in the closeEvent implementation, it could be removed when TT has
fixed the issue.

OK, let them only in the closeEvent implementation but put a FIXME explaining the issue there.


+ // check for X11 -geometry option
+        if (argv[i] == string("-geometry"))
+            geometryOption_ = true;
+
I have read somewhere that QApplication already understand this option.
How would this interact with your code here?


Qt4::QApplication handles the -geometry option in the ctor,
Qt3::QApplication handles it when calling setMainWidget.

So when we use Qt4 we must know if we should resize the window
after constructing, and therefore we must knowif
there was a geometry option.

By this way both front ends uses the same logic:
Construct with geoemtry size/pos (if any) and only resize/move
when there was no geometry option.

I understand, thanks.

Abdel.

Reply via email to