Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Dear afv,

Please, consider the following points when processing this issue (as agreed during our meeting yesterday):
1. Find the reason for incorrect system window size after using maximize/restore buttons in the Qt 5.5 sources.
2. Describe the Qt 5.5 behavior specifics that lead to incorrect behavior of OCCT viewer in this issue, and explain why the involved OCCT API is incompatible with Qt behavior.
3. NEW: In order to avoid conflicts between Qt 5.5 and OS-specific OCCT API (WNT_Window, Xw_Window), it is proposed to wrap the view widget into a custom Aspect_Window descendant class at the application level and retrieve window geometry from QWidget.

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Sep 2 17:26:57 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

I created two branches:
1) CR26603 - branch for tutorial sample.
2) CR26603_1 - branch for occt products.

Branches CR26603 (occt) and CR26603_1 (occt-products) reviewed with the following remarks.

General
=====================================
- Commit comment in products branch CR26603_1: read carefully the OCCT contribution workflow regarding the correct commit messages.
- What about Qt platform plugins? No relevant changes made to env.bat.
- It is proposed to add QTDIR variable explicitly to env.bat and subsequently add %QTDIR%/bin to PATH like in env.sh under Linux.

samples/qt/Common/src/CrossWindow.cxx
=====================================
- Choose class names more carefully! CrossWindow is difficult to understand. In a Qt-based application, OcctWindow or WidgetWrapper would be more relevant.

- Destroy() is too complicated - can be simplified as follows:
+void CrossWindow::Destroy()
+{
+ myWidget = NULL;
+}

- What is the purpose of myXLeft, myYTop, myXRight and myYBottom data members? My understanding is that they are used to store previous widget geometry and determine the type of resize in DoResize(). Meanwhile, widget size and position is returned using the current widget geometry - i.e. myWidget->rect(). This is done correctly in some methods but not in Position(), so please correct it. Else please explain the role of these data members and the way they are used.

- Check the OCCT coding rules regarding placement of parentheses in if-statements, expressions and function calls.

- NativeParentHandle(), NativeHandle(): function header comments missing.

- NativeParentHandle(): will crash if myWidget->parentWidget() returns 0.

samples/qt/Common/src/CrossWindow.h
=====================================
- Remove DEFINE_STANDARD_HANDLE macro and forward declaration of the class, refer to Standard_Handle.hxx for details

- Once again, consider a different, more understandable class name

- Revise the class description:
  - Avoid "we" usage, impersonal sentences or third-person sentences are preferred in source code documentation (using "it" or "the class" as the subject).
  - Remove irrelevant phrases like " When building this application" - only when building? Not at runtime? It is enough to say "With Qt 5.x, ..."
  - Check carefully what you wrote - e.g. I do not understand "when you change the window size using the operating system, we will receive incorrect data, as part of our application" - it has nothing to do with what we understood during Qt debugging, we did not change the window size using Win32 API but we queried the window size using Win32 API.
  - Make sure you do not mix "slot" and "event"
  - I would rather say "an implementation of Aspect_Window interface based on QWidget API" instead of "cross-platform Window"
  - "use only the internal data Qt" - we do not use internal Qt data at all. Your class relies on window geometry information provided by Qt API.
  - "( in our case this is the widget where the window based )" - completely unclear, do you mean "the Qt widget where a 3D view content is displayed"?
  - Please check English grammar! Ask our technical writers for help if necessary.
   
- Position() method:
  - Remove Standard_EXPORT macro - I guess this class' methods need not to be exported because you build an executable application rather than a DLL
  - Correct indentation
  
samples/qt/Common/src/View.h
=====================================
- myDrawRect field declaration - correct indentation

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 4 11:53:42 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x
    Fixed remarks.

Author: Author
Date: Wed Sep 2 17:51:57 2015 +0300

    minor remark

Author: Author
Date: Wed Sep 2 17:49:58 2015 +0300

    minor remark

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 4 13:57:35 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Branch CR26603 reviewed with the following remarks:

samples/qt/Common/src/OcctWindow.cxx
=====================================
OcctWindow::NativeParentHandle() - incorrect indentation, see below:

+void OcctWindow::Destroy()
+{
+  myWidget = NULL;
+}
+
+// =======================================================================
+// function : NativeParentHandle
+// purpose  :
+// =======================================================================
+Aspect_Drawable OcctWindow::NativeParentHandle() const
+{
+  QWidget* aParentWidget = myWidget->parentWidget();
+  if ( aParentWidget != NULL )
+       return (Aspect_Drawable)aParentWidget->winId(); <---!!!!
+  else
+       return 0;                                       <---!!!!
+}

OcctWindow::Position() - incorrect indentation within the argument list, see below:

+void OcctWindow::Position ( Standard_Integer& theX1, Standard_Integer& theY1,
+                             Standard_Integer& theX2, Standard_Integer& theY2 ) const <--!!!

samples/qt/Common/src/OcctWindow.h
=====================================
Class documentation - please replace with the text below:

OcctWindow class implements Aspect_Window interface using Qt API as a platform-independent source of window geometry information. A similar class should be used instead of platform-specific OCCT classes (WNT_Window, Xw_Window) in any Qt 5 application using OCCT 3D visualization.

With Qt 5, the requirement for a Qt-based application to rely fully on Qt public API and stop using platform-specific APIs looks mandatory. An example of this is changed QWidget event sequence: when a widget is first shown on the screen, a resize event is generated before the underlying native window is resized correctly, however the QWidget instance already holds correct size information at that moment.
The OCCT classes acting as a source of window geometry for V3d_View class (WNT_Window, Xw_Window) are no longer compatible with changed Qt behavior because they rely on platform-specific API that cannot return correct window geometry information in some cases. A reasonable solution is to provide a Qt-based implementation of Aspect_Window interface at application level.

virtual void Position() - incorrect indentation within the argument list (not corrected after previous code reviewing), see below:

+  virtual void Position( Standard_Integer& theX1, Standard_Integer& theY1, 
+                                                Standard_Integer& theX2, Standard_Integer& theY2 ) const; <--!!!

samples/qt/Common/src/View.h
=====================================
Incorrect indentation - see below (not corrected after the previous code reviewing):

     bool                            myIsAntialiasingEnabled;
 
-    bool                            myFirst;
-    bool                                       myDrawRect;           // set when a rect is used for selection or magnify 
+    bool                                   myDrawRect;           // set when a rect is used for selection or magnify 
     Handle(V3d_View)                myView;

samples/qt/IESample/env.bat
=====================================
Should be corrected similarly to samples/qt/Tutorial/env.bat

Hi, I have checked the fix. Unfortunately it seems not to work here. I am using Qt 5.4.2 and the OCC community edition. Also I have modified the Qt sample so that it is not exactly your code anymore, but it is extremely similar.

The resizing problem still happens here. The actual 3D view does not fit to the window. Would you mind using a gradient background? Then you can easily see whether the 3D view is correctly resized.

I use the following:

------------------
Quantity_Color mainColor = Quantity_Color(51.0 / 255.0, 51.0 / 255.0, 102.0 / 255.0, Quantity_TOC_RGB);
Quantity_Color gradientColor = Quantity_Color(171.0 / 255.0, 171.0 / 255.0, 192.0 / 255.0, Quantity_TOC_RGB);
myView->SetBgGradientColors(mainColor, gradientColor, Aspect_GFM_VER, Standard_False);
------------------

Another problem is that the fix seems to be incompatible with Qt 4. But I can use

#if (QT_VERSION < QT_VERSION_CHECK(5,0,0))
...
#endif

to make my application work with Qt 4 and Qt5.

I am looking forward to your answer.

To make my point more clear, let me give you a number of steps to reproduce my problem:

1. Start application. The application window is maximized and the document window is maximized within the application window.
2. "Restore Down" the document window (the inner window, not the application window). "Restore Down" is the opposite action of "Maximize" and it is done by clicking onto the same window icon.
3. "Maximize" the document window again. Now the 3D view is smaller than the document window. Its size is the same as returned by the sizeHint() method from the Qt window class. Please see the attached file Attachment 4 (JPG).

Hello Benjamin,

Can you please reproduce the situation illustrated by Attachment 4 (JPG) using standard OCCT Qt samples (Tutorial or IESample) and the OCCT development snapshot form CR26603 Git branch? Without this your comment does not look relevant.

And thanks for the remark regarding Qt 4.x compatibility, we will take it into account.

I will try to reproduce the error with standard OCCT samples. This will take a while and I am not sure whether I can find the time to finish it.

Still changing the background and trying maximization/restoring of the document window should just take a few minutes. Therefore I would like to ask you to try that. I am quite sure that you will see the same errors as described above.

Naturally, we have already tried at our side and found no problem, resizing seems to work correctly with the patch from CR26603 branch.
I mean not only the standard OCCT samples but also the OCCT version you use, please see the beginning of your comment above (#0026603).

To not make you wait: I will stick to Qt 4. Never mind, you can mark the issue as resolved.

I have the same issue even if the patch CR26603 is applied in QT5.5. It just reproduce at the first time to show up the window. it works after resizing the window manually.

Hello Vico,

Please, confirm that you use the following environment:
- Qt 5.5
- OCCT + the standard Qt sample (Tutorial or IESample) built from sources taken from CR26603 branch
I need to make sure that we use exactly the same OCCT snapshot.
Thanks a lot in advance!

Dear san,

The environment in my side: Qt 5.5 + OCCT6.9 + CR26603. But I did not try that with occt Qt sample, i did it in my own way with patch CR26603. All build from source codes.

Dear Vico,

I had not managed to compile OCCT with MSVC 2013 (compile error about overloads having similar conversions).

But if you have already compiled OCCT, could you check the OCCT Qt sample? If you can reproduce the bug, but san cannot (let's assume he has really tried hard to do so), then can it have to do anything with our graphics card? Mine here is NVIDIA Quadro 4000.

Thank you!
Benjamin

Where to get the QT sample for QT5.x? the default QT sample is still packaged in QT4.

Dear all,

Most likely, the principal difference in behavior of the current OCCT master and OCCT 6.9.0 is due to the following patch for #26392:
http://git.dev.opencascade.org/gitweb/?p=occt.git;a=commitdiff;h=[revision removed]#patch26

Thus if you take the application patch from CR26603 and the modification for OpenGl_Window.cxx from #26392, everything might work as expected. Can you please check?

If you have checked out the new branch, then in the folder occt\samples\qt\Common\src you will find the files OcctWindow.cxx and OcctWindow.h. This is the new adaption to Qt 5. Then you could compile the sample occt\samples\qt\Tutorial and check whether document window maximation works.

Thank you for the tip, san, I will try that.

Just to remind that we are waiting for further feedback regarding this issue.
Does OCCT 6.9.0 with the patched OpenGl_Window.cxx works as expected in your environment?

I am compiling an updated OCE version right now and I hope that I can give you further information this evening.

Dear san,

your tip was correct. If I apply the changes from the patch #26392, the document window maximization/restoration problem disappears! Thank you very much for your great work!

Still I have another problem! :-( It seems to be also related with Qt 5 and the way the OpenGL window is created or destroyed. Unfortunately it is very blurred. Here is the description:

When I close document windows and open new ones, sometimes it happes, that the document window content is not updated correctly. This is a problem of that document window and it stays like that during the window lifetime. When I resize the window manually, the content is not correctly redrawn. Yet sometimes (especially if I move the window out of the screen and back again) content updates may happen. I will attach a screenshot called Attachment 3 (JPG). There the left document window has this update problem, while the right document window is okay.

I could spot one difference between windows being updated correctly and "damaged" windows:
When I create the first document window of my QMdiArea or if I close a document window, I get this error message:

TKOpenGl.WinSystem | Type: Error | ID: 6 | Severity: High | Message:
  wglMakeCurrent() has failed. Das Handle ist ung�ltig.

The german sentence means: "The handle is invalid".

This error message did not appear when I was using Qt 4.

"Normally" this error message appears exactly once during document window creation or closing. But sometimes it appear six times. In that case the window is "damaged" and the updates will not be performed correctly.

Have you ever seen an error message like this or have you also had that problem? Is there another patch branch fixing it?

I know that this is a terrible error description. But unfortunately this is a sporadic problem (could it have to do something with concurrent code and timing problems during document window creation?).

Thank you very much for your answer.

Benjamin

Benjamin,

Thanks for confirming that the window resizing issue is resolved!

Now let me sum up the new problem:
- It is not reproduced with help of an official OCCT release or the current OCCT development version
- It is not reproduced with help of the standard OCCT sample application
- It is not reproduced in a regular and deterministic way

In this situation, one approach that could be recommended is migrating your application to an official OCCT release (http://www.opencascade.com/content/download-center) or to the OCCT development version (http://dev.opencascade.org/index.php?q=home/resources), checking if the problem still exists, and trying to reproduce it using the standard OCCT sample application (or providing another reproducer) so that the OCCT development team be able to debug the issue.

Alternatively, you can apply for our commercial customer support services: http://www.opencascade.com/content/technology-support. Like this, our experts will be able to help you most efficiently.

Dear afv,

Please clean up the pending reviewer's remarks (if any) and send the patch back for the final reviewing and testing.

san,

sorry for annoying you that much!

[signature removed]
Benjamin

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 18 09:22:38 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Fixed all remarks for note 0045495.

New files have been lost in the patch, please add them back - and double-check the patch contents in general.

And what about Qt 4 compatibility?

To sum up TODO:
- Remarks from #0026603 - seems to be done
- Missing files (OcctWindow) + verify the patch contents and make sure all your local changes are included
- Qt 4 compatibility (see #0026603), do the same with the product samples please.

Some more TODOs - without them the build environment for the Qt samples is broken:

- Rebase CR26603 branch onto the current master
- Get a clean local copy of OCCT from this branch and build it using CMake - like this, you will obtain new OCCT binary tree structure where %CASROOT%\msvc.bat is no more available. A quick solution is to copy msvc.bat from adm\templates by genproj.bat for each Qt sample. Later on, Qt samples will be migrated to CMake.
- Update the Qt samples for this new directory structure and check compilation and execution.

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Sep 30 10:55:24 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Author: Author
Date: Wed Sep 30 10:48:53 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Author: Author
Date: Mon Sep 28 10:23:48 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Author: Author
Date: Mon Sep 28 09:49:41 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Mon Oct 5 13:36:07 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Before check branch CR26603_2 , need to check branch CR26739 in OCCT and OCCT Products

Please add the patch for Linux.

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 8 09:33:47 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Dear afv,

What is the current status of this issue? Any specific problems under Linux?

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Branch CR26603_2 in occt and occt-products reviewed without remarks.
Please test Qt samples based on IESample with Qt 5.5.0 under all supported operating systems.

Dear Commenter 1,
Branch CR26603_2 from occt git-repository (and CR26603_2 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
[revision removed]

Number of compiler warnings:

occt component :
Linux: 13 (13 on master)
Windows: 0 (0 on master)

products component :
Linux: 39 (39 on master)
Windows: 0 (0 on master)

There are compilation errors in occt Qt samples on windows platform:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26603-2-CR26603-2/job/CR26603-2-CR26603-2_build_occt_qt_samples_window/1/parsed_console/
1>d:\builds\vc10\CR26603-2-CR26603-2-occt-64\inc\Font_FTLibrary.hxx(23): fatal error C1083: Cannot open include file: 'ft2build.h': No such file or directory

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Dear san,

Please review. This is minor fix - in base branch 0026739 qt samples VoxelDemo was removed at all.

Branch CR26603_2 reviewed without remarks - please test the Qt samples with Qt 5.5 on all platforms, paying attention to different ways of resizing a 3D view window (minimize, maximize, resize manually by dragging a corner or a border).

Dear afv,
could you please rebase branch CR26603_2 on current master of occt git-repository, there are conflict files.
Dear afv,
could you please rebase branch CR26603_2 on current master of products git-repository, there are conflict files.

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Mon Oct 19 15:37:50 2015 +0300

    0026603: Problem with maximization and normalization document windows in Qt samples with Qt 5.x

Branch [archived branch] has been updated forcibly by Author.

[revision removed]

Dear Commenter 1,
Branch CR26603_3 from occt git-repository (and CR26603_3 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
[revision removed]

Number of compiler warnings:

occt component :
Linux: 8 (8 on master)
Windows: 0 (0 on master)

products component :
Linux: 39 (39 on master)
Windows: 0 (0 on master)

There are not compilation errors in occt Qt samples on Windows platform:
There are not compilation errors in products Qt samples on Windows platform:

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 92098290 / 92270380 [-0.19%]
Total CPU difference: 19745.289999999764 / 19657.70999999973 [+0.45%]
products component :
Total MEMORY difference: 26258128 / 26295062 [-0.14%]
Total CPU difference: 7741.659999999993 / 7414.720000000001 [+4.41%]

Testing on Windows:
occt component :
Total MEMORY difference: 58026127 / 58036786 [-0.02%]
Total CPU difference: 18297.326089799157 / 17972.048404698966 [+1.81%]
products component :
Total MEMORY difference: 17164123 / 17167618 [-0.02%]
Total CPU difference: 5648.983411199966 / 5655.301451699955 [-0.11%]

There are no differences in images found by testdiff.

Branch [archived branch] has been deleted by Commenter 5.

[revision removed]

Branch [archived branch] has been deleted by Commenter 5.

[revision removed]

Branch [archived branch] has been deleted by Commenter 5.

[revision removed]

Branch [archived branch] has been deleted by Commenter 5.

[revision removed]