coin3d / pivy

python bindings to coin3d
ISC License
53 stars 39 forks source link

example 10.2 not working with soqt #18

Closed looooo closed 7 years ago

looooo commented 7 years ago

PySide.QtCore.Qevent is not properly wrapped: <pivy.gui.soqt.QEvent; proxy of <Swig Object of type 'QEvent *' at 0x00000187BBBAA8D0> >

InventorMentor commented 7 years ago

Many thanks! First of all it is very encouraging to folllow all your recent progress providing soqt also for pivy-win64-py3. From an Inventor Mentor point of view, once example 10.2 will work and an SoQtExaminerViewer is embeddable into a pyside application, you could declare victory and bump pivy's version number to 1.0 :-) but maybe this is just me dreaming of a not too distant future ...

I'm currently using winpython 3.6, therefore, I could not apply your prebuild pyside package above. Also following the discussion in conda-forge/pyside-feedstock#32 size_t is obviously preferable to unsigned long long since it will preserve 32-bit compatibility. I still don't get why applying the same fix to https://github.com/krrr/PySide doesn't solve the wrapInstance issue for me. Could you please point me to your pyside sources so that I can use them to build a patched pyside-win64-py36 myself?

looooo commented 7 years ago

thanks very much. I am pretty sure once we have solved the 10.2 problem we will find other problems too. But I would like to release a 0.6.2 version, and therfore at least some issues should be solved.

regarding the source: it's all inside the meta.yaml file: https://github.com/looooo/pyside-feedstock/blob/master/recipe/meta.yaml#L9

best is to:

  1. clone the recipe
  2. download the source
  3. extract the source
  4. git init
  5. git apply <pathes>

at least conda does these steps automatically...

InventorMentor commented 7 years ago

Many thanks, I'm going to give this recipe a try tomorrow. Having all functionality-wise relevant inventor mentor examples working now (at least with soqt, eventually also even with pivy-quarter, many thanks for your recent commit, I'll definitely check this out tomorrow) appears to be a very good baseline for a pivy release (feature completeness demonstrated on both py2/py3 and linux/windows 32/64.

Just one more question out of my memory: have you already fixed SoBase.i with size_t as described in one of my posts above and also I think that the original soqt 1.6.0a sources need to be patched correspindingly to wrapInstance in a few places. If you grep for unsigned long or maybe better just for long, you will find in two or three files (SoQtComponent.*, etc) down-casts of memory addresses used in a dictionary for QWidget/QEvent lookup internally in SoQt. This is expected to fail on win64, so again replacing these casts wih ones to size_t instead should be the correct and portable thing to do. I can look up the details of my changes to soqt tomorow if you want. hopefully, this will finally close the remaining issue with 10.2 ...

looooo commented 7 years ago

found one in SoBase: https://github.com/looooo/pivy/blob/636ddbd3a47b8aef9a6277643287cf2e97c14156/Inventor/misc/SoBase.i#L7

but the other files are coin related... I just started a poll at freecad to fork coin, So maybe we can patch this there... :-) https://forum.freecadweb.org/viewtopic.php?f=10&t=22714

looooo commented 7 years ago

I think the remaining problem of 10.2 is related to my implementation of get_typename: https://github.com/looooo/pivy/blob/master/interfaces/soqt.i#L114 This returns for example: class QMouseEvent but actually it should return only QMouseEvent So somehow the first 6 characters should be removed. I tried with std::string and string.erase but wasn't successfully. Maybe you have got an idea how to remove the first characters.

InventorMentor commented 7 years ago

I'm not really convinced that character string operations are the most efficient thing to do when handling events, though modifying pointers to the starting character of a string should be very efficient (however, for safety concerns, this introduces assumptions of non-unicode C-style strings always being prefixed with string "class "; the first assumption [1 byte/char strings] should be pretty safe since the signature of the function is dealing with char*, anyway; however, I'm not so sure about the second assumption being always met; could there ever occur empty strings for example? --> then a crash or undefined behaviour would result); if both assumptions are always met, my suggestion is to use something like in the second line of the following for returning the result (just be adding +6 in the return statement):

const char* simpleNonUnicodeAsciiCStyleString = "class QMouseEvent";
const char* shortenedVersion = simpleNonUnicodeAsciiCStyleString + 6;  // 6 == sizeof("class "), 1 byte/char
printf("shortened version= \"%s\"", shortenedVersion); // trimmed from left by six bytes (ascii characters)
looooo commented 7 years ago

I knew you will have a solution for this : ) Just tried it and it works quite well. I have gone through all Mentor examples on windows and made a list of the not working ones: https://github.com/looooo/pivy/issues/20

If you can reproduce the working 10.2 example I think we can close this issue : )

looooo commented 7 years ago

and there is a new one! testing py36-> https://travis-ci.org/looooo/pivy/jobs/237436673#L665

InventorMentor commented 7 years ago

May Betrand Meyer (of "programming by contract" fame, i.e. explicit contracts between caller and callee, explict specification of pre- and postconditions for subroutines) forgive us for just making the second assumption, but at least we could put the burden on the caller by makig these assumptions explicit in a comment (taking the role of such a contract :)

InventorMentor commented 7 years ago

With fixing 10.2, I'm unfortunately still not there yet. I've built PySide according to your recipe above (and had to make some destructors public for that in Qt-4.8.7 on the way), but I still receive a truncated 32-bit this-pointer in line 392 of file soqt-1.6.0\src\Inventor\Qt\SoQtComponent.cpp

PRIVATE(this)->parent->installEventFilter(PRIVATE(this));

I still get the folllowing: Unhandled exception at 0x000000006C61ADFF (QtCore4.dll) in python.exe: 0xC0000005: Access violation reading location 0x0000000073969D58. Callstack:

soqt1.dll!SoQtComponent::SoQtComponent(QWidget * const parent, const char * const name, const int embed) Line 393   C++
soqt1.dll!SoQtGLWidget::SoQtGLWidget(QWidget * const parent, const char * const name, const int embed, const int glmodes, const int build) Line 179 C++
soqt1.dll!SoQtRenderArea::SoQtRenderArea(QWidget * parent, const char * name, int embed, int mouseInput, int keyboardInput) Line 887    C++

PS: In file SoQtComponent.cpp I now cast to SbDictKeyType instead of size_t (originally just long truncating QObject*) which reports me on win64 when calling sizeof(SbDictKeyType) = 8 bytes with SbDictKeyType being defined in coin3d's Inventor/SbDict.h as typedef uintptr_t SbDictKeyType; // aren't uint64_t-pointers memory aligned? I'm not sure whether this is also the case with void* or in this case QObject*, respectively.

My changed typecasts in SoQtcomponent.cpp are now line 144: SbBool b = SoQtComponentP::cursordict->find((SbDictKeyType)cc, qc); line 191: SoQtComponent.cpp: SoQtComponentP::cursordict->enter((SbDictKeyType)cc, c); but still get the same problem above with 10.2:

looooo commented 7 years ago

I can confirm the problem still exists in win8 and win10. 10.2 and quarter-examples work on win7 (at least for me). on win8 at least the quarterWidget was working... On windows 10 it seems quarter does not work either. Different windows versions seems to be much more worse then different linux distros... ; )

InventorMentor commented 7 years ago

It's sort of interesting that 10.2 actually works for you under win7. Could you please give furher details about your working configuration, e.g, such as ... Win7 64 bit with 8 GB RAM WinPython 3.6.1 - 64 bit SoQt 1.6.0a with or without patch ... build with recipe using msvc14 Pivy 0.6.2 branch master, commit .... Coin3d 4.0.0a with/without patch .... using msvc14 shared release build Qt 4.8.7 with/without patch ... msvc 14 build PySide 1.2.4 build using recipe ...

I reckon that the win64 issue is with pivy's SoQt binding and/or eventually also with SoQt itself. My shiboken built with your PySide recipe gives me now valid 64 bit cppPointers for QObjects created with PySide under 64 bit win10. Maybe win7 64 bit allocates some application addresses still below 4 GB so that a pointer truncation to 32-bit does not necessarily crash the application while 64 bit win8 / win10 assign application pointers to virtual addresses greater than 32 bit (4 GB), while 32-bit addresses below 4 GB are reserved for the operating system itself (this used to be 2 GB only by default in 32 bit Windows XP but could be changed via a registry setting to 1 GB, so applications had a maximum of 2 or 3 GB available) but I'm just speculating and still have to research that.

PS: Can you actually import shiboken in your working win7 configuration or is your soqt binding there making use of its swig-based object wrapping fallback instead?

looooo commented 7 years ago

win7 64 bit 16gb ram for version info, source,... please have a look at the build recipes which can be found in the FreeCAD_Conda repository: https://github.com/looooo/FreeCAD_Conda Basically the sources are taken from the pivy release. soimage, soqt are slightly changed. Coin is patched in the recipe.

In my testenvironment I have these packages:

boost-cpp                 1.64.0                   vc14_0  [vc14]  conda-forge
ca-certificates           2017.4.17                     0    conda-forge
certifi                   2017.4.17                py35_0    conda-forge
coin3d                    4.0.0                    vc14_4  [vc14]  freecad
icu                       58.1                     vc14_1  [vc14]  conda-forge
jpeg                      9b                       vc14_0  [vc14]  conda-forge
libiconv                  1.14                     vc14_4  [vc14]  conda-forge
libpng                    1.6.28                   vc14_0  [vc14]  conda-forge
libtiff                   4.0.6                    vc14_7  [vc14]  conda-forge
libxml2                   2.9.4                    vc14_4  [vc14]  conda-forge
libxslt                   1.1.29                   vc14_4  [vc14]  conda-forge
openssl                   1.0.2h                   vc14_3  [vc14]  conda-forge
pip                       9.0.1                    py35_0    conda-forge
pivy                      0.6.2                py35_dev_3  [vc14]  freecad
pyside                    1.2.4               py35_vc14_6  [vc14]  freecad
python                    3.5.3                         3    conda-forge
qt                        4.8.7                    vc14_7  [vc14]  conda-forge
setuptools                33.1.1                   py35_0    conda-forge
simage                    1.7.0                    vc14_0  [vc14]  freecad
soqt                      1.5.0                    vc14_0  [vc14]  freecad
vc                        14                            0    conda-forge
vs2015_runtime            14.0.25420                    0    conda-forge
wheel                     0.29.0                   py35_0    conda-forge
wincertstore              0.2                      py35_0    conda-forge
zlib                      1.2.11                   vc14_0  [vc14]  conda-forge

I tested the exactly same repository on win8 and win 10.

To get the same environment on another machine you can do the following:

InventorMentor commented 7 years ago

Thanks, I will do that. I've got access to a 64 bit win7 machine, too. If it works my plan is to replace various modules in the hope that one of these will produce the bug again to narrow down the culprit. Before I do that could you please verify that you can directly import shiboken on this win7 installation? The swig-fallback in soqt seems to work much better across windows versions for most examples than the shiboken-based default wrapper of soqt ...

looooo commented 7 years ago

import shiboken works for:

actually I made some mistakes previously on windows10. Quarter does work for me. Only 10.2 without coin argument crashes. (Maybe I have installed an older version...) Also running 10.2 with the coin argunment prints a Overflowerror, which win7 does not.

OverflowError
TypeError: You need a shiboken-based type.
TypeError: You need a shiboken-based type.
TypeError: You need a shiboken-based type.

So maybe again some problems with pyside.

looooo commented 7 years ago

some further observation in win10:

InventorMentor commented 7 years ago

On 64-bit win10, example 10.2 only works as expected for me with option coin when using the swig-based soqt-binding (without an installed shiboken). Without option coin and without shiboken installed manually to Lib\site-packages (to disable the direct import shiboken expected by pivy instead of from PySide import shiboken as provided by the default PySide installation) I receive an

AttributeError: 'QEvent' object has no attribute 'type'
Traceback (most recent call last):
  File "10.2.setEventCB.py", line 140, in myAppEventHandlerQt4
    if anyevent.type() == QEvent.MouseButtonPress:

On 64-bit win10, with shiboken installed manually to Lib\site-packages running example 10.2 gives me with option coin random startup crashes of the application due to 32-bit pointer truncation as reported above and only sometimes when I start 10.2 the render area actually appears but then I get the OverflowError / shiboken-based type error message you reported above. Otherwise, the example is usable when it actually made it to startup. Omitting option coin [as the expected default use case for example 10.2 with shiboken manually installed to Lib\site-packages], however, starts up only occassionaly (randomly) but then I'm unable to draw in the render area and it prints the error message

AttributeError: 'QEvent' object has no attribute 'type'
OverflowError
Traceback (most recent call last):
  File "10.2.setEventCB.py", line 140, in myAppEventHandlerQt4
    if anyevent.type() == QEvent.MouseButtonPress:

In random cases, when example 10.2 does not start up at all with shiboken and crashes directly on startup for me, then the attached post-mortem debugger halted at an exception in this constructor call

SoQtGLWidget::SoQtGLWidget(QWidget * const parent,
                           const char * const name,
                           const SbBool embed,
                           const int glmodes,
                           const SbBool build)
  : inherited(parent, name, embed),
    waitForExpose(TRUE),
    drawToFrontBuffer(FALSE)

however, then the pointers like parent can not be inspected from the debugger with message Variable is optimized away and not available.

PS: Since under 64-bit win10 example 10.2 with option coin apparently always works as expected when import shikoken is not available (just with the swig-based 'fallback' wrapping of pivy.gui.soqt) , I reckon we may exclude libraries Coin3D and SoQt as the source of this issue. Testing a corresponding native C++ example 10.2 implementing a C++ version of myAppEventHandlerQt4(myRenderArea, anyevent) also works reliably and as expected on 64-bit win10, providing further evidence that this issue is not with native Coin3D 4.0.0a, SoQt 1.6.0a, or Qt-4.8.7, but must be with pivy, its's shiboken-based soqt-binding, pyside, or shiboken.

looooo commented 7 years ago

just came across these two warnings:

pivy\coin_wrap.cpp(5467): warning C4311: 'type cast': pointer truncation from 'SoBase *' to 'long'
pivy\coin_wrap.cpp(5467): warning C4302: 'type cast': truncation from 'SoBase *' to 'long'
InventorMentor commented 7 years ago

Test case: running failing example 10.2 on 64-bit win10 with available import shiboken multiple times until it randomly proceeds to show the render area after multiple attempts; The OverflowError reported above is printed in the body of the following failing if statement attempting to call shikobken_wrapinst_func in line 43 of file pivy/Inventor/Qt/SoQtRenderArea.i:

if (!(qev = PyEval_CallObject(shiboken_wrapinst_func, arglist))) {
            PyErr_Print();  // prints "OverflowError"
}
looooo commented 7 years ago

so this is again a problem with the pointer-size? Allthough it is now set to size_t. regarding this site: https://msdn.microsoft.com/en-us/library/323b6b3k.aspx size_t can be unsigned __int64 or unsigned integer, depending on the target platform...

looooo commented 7 years ago

unsigned long long has the same problem. If I understand it correctly: If the wrapping fails (OverflowError) the example works because for the minimal example no pyside type is necesarry. If the wrapping does work, there is a crash (because of wrongly wrapped qt-object)

InventorMentor commented 7 years ago

Working backwards from line 43 in SoQrarenderArea.i, I'll have to insert printf statements to see what is going on and maybe to test assumptions and spot differences against a working scenario on win7 or eventually linux64, since I do not have a debug build of py3, pivy, qt, ... you name it .... therefore, the good old printf debugger should be my best bet ... some Maya folks apparently had a very similar OverflowError: need a shiboken based type ... and recommend as a solution to import shiboken only as from PySide import shiboken - the printf debugger will not be too easy to implement, however, because of the convoluted PyObject structures involved. Maybe you can verify that shiboken gets imorted correctly into pivy on win10 ...

cgohlke commented 7 years ago

... this didn't fix the Overflow issue with example 10.2, however. Therefore, instead of using Chris Gohlke's prebuild win64 wheel for PySide 1.2.4, I decided to build one myself including the shiboken 64-bit address patch in wrapInstance above by ...

FWIW, I have updated my PySide wheels with the shiboken 64-bit address patch. The OverflowError should be fixed.

InventorMentor commented 7 years ago

Dear Chris Gohlke, thank you very much indeed, I'll be most happy to try this. I can confirm that the issue for me on win10 was shiboken.wrapInstance as I can now confirm on my machine using a minimal test case taken from https://bugreports.qt.io/browse/PYSIDE-103

from PySide import QtCore, shiboken
shiboken.wrapInstance(0xFFFFFFFF, QtCore.QObject)  # Overflow error --> largest 32-bit address, howevsser, this should definitely work on all systems
shiboken.wrapInstance(0x100000000, QtCore.QObject) # Overflow error --> an actual 64-bit address > 4 GB (the first non 32-bit address)

However, a bit surprisingly, a slightly smaller 32-bit address shiboken.wrapInstance(0xFFFFFFF, QtCore.QObject) works for me on 64 bit win10.

Now I can test this against your wheel .... I reckon you made my day :-)

cgohlke commented 7 years ago

Works for me:

Python 3.6.1 (v3.6.1:69c0db5, Mar 21 2017, 18:41:36) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from PySide import QtCore, shiboken
>>> shiboken.wrapInstance(0xFFFFFFFF, QtCore.QObject)
<PySide.QtCore.QObject object at 0x00000143E4F58288>
>>> shiboken.wrapInstance(0x100000000, QtCore.QObject)
<PySide.QtCore.QObject object at 0x00000143E4F583C8>
InventorMentor commented 7 years ago

finally, this minimal test case above also works for me, too :-) Thank you very much indeed! ... now, I'm looking forward to test example 10.2 and examinerEmbed4.py (testing the embedding of a Coin3D / OpenInventor examiner viewer into a PySide application that also failed on 64-bit win10 to this end)

InventorMentor commented 7 years ago

Unfortunately, running example 10.2 still fails for me with an Unhandled exception at 0x00000000541AADFF (QtCore4.dll) in python.exe coming from a 32-bit address on my 64-bit win10 machine using 64-bit WinPython 3.6.1. QtCore.dll is on my system a self-compiled 64-bit msvc14 release dll and lives here (also in the system's search path): C:\Qt\4.8.7_msvc14\bin\QtCore4.dll

InventorMentor commented 7 years ago

after recompiling pivy, I interestingly receive now a different error message than before: OverflowError: can't convert negative int to unsigned

InventorMentor commented 7 years ago

@looooo you may want to try and add the following script to pivy/tests:

# shiboken_64bit_test.py
from PySide import QtCore
import shiboken  # as expected by pivy (instead of the default "from PySide import shiboken")

q = QtCore.QObject()
ptr = shiboken.getCppPointer(q)
print("CppPointer to an instance of PySide.QtCore.QObject = 0x%016X" % ptr[0])

# None of the following is expected to raise an OverflowError on 64-bit systems
shiboken.wrapInstance(0xFFFFFFFF, QtCore.QObject)  # largest 32-bit address
shiboken.wrapInstance(0xFFFFFFF, QtCore.QObject) # a regular, slightly smaller 32-bit address
shiboken.wrapInstance(0x100000000, QtCore.QObject) # an actual 64-bit address (> 4 GB, the first non 32-bit address)
shiboken.wrapInstance(0xFFFFFFFFFFFFFFFF, QtCore.QObject) # largest 64-bit address
looooo commented 7 years ago

added your test.

just a side note:

import shiboken # as expected by pivy (instead of the default "from PySide import shiboken")

I think shiboken should be importable directly. At least that was my conclusion from reporting it to conda: https://github.com/conda-forge/pyside-feedstock/issues/31

the test fails for win10 and ubuntu at:

ERROR: testAdresses (__main__.ShibokenTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "coin_tests.py", line 1226, in testAdresses
    shiboken.wrapInstance(0xFFFFFFFF, QtCore.QObject)  # largest 32-bit address
OverflowError

ps.: same is also true for win7. does this work for you?

InventorMentor commented 7 years ago

Yes, it works for me, but I don't know why. It should have worked with the patch for PySide-103, however, it didn't ... both my patched build based on krrr's PySide and according to your PySide recipe failed for me regarding shiboken wrapInstance. However, Chris Gohlke, suddenly dropped by to the rescue yesterday and provided a python wheel with a PySide that ships with a working 64-bit shiboken for windows ... but there is no source code and I can live without it for my needs .... I reckon FreeCAD's PySide, however, will also need this fix... apparently Autodesk is also publishing a fixed PySide on its website for Maya 2015.

PS: I've also found the bug :-) example 10.2 and examinerEmbed4 now happily work for me on 64-bit win10 ... I still have to investigate things a bit further and need to tidy up ...

InventorMentor commented 7 years ago

btw ... the bug was a type downcast error from a 64-bit address (of a void*) to a (signed 32-bit) int instead of the needed and plattform-independently correct size_t. There may be more such bugs lurking around ... I'll check that and will post a fix within the next couple of days!

looooo commented 7 years ago

wow, that are very good news. thanks for all the work!

InventorMentor commented 7 years ago

Sorry for my late reply. I needed to take a little break away from pivy after diving deep into this issue. Please find my patched versions of pivy/Inventor/Qt/SoQtRenderArea.i https://www.dropbox.com/s/6v0mmvk3638vdq8/SoQtRenderArea.i?dl=0 and pivy/interfaces/soqt.i https://www.dropbox.com/s/i2eydxzeixctc80/soqt.i?dl=0 ready for you to download under the above two URLs. Please copy their file contents over their existing versions in your current git clone on branch master and please review a git diff of my changes and check whether you are happy with them and would agree to keep them. All relevant examples including 10.2 (except just examples 9.1 and 12.4, but I'll write seperate issues for these later) now work happily as expected for me under 64-bit Windows 10. Please note that the complete fix to this issue also requires a working 64-bit version of shiboken for windows (e.g. as provided by Chris Gohlke's recent PySide wheels).

I feel it would be really good if we knew how the build these patched wheels ourselves just in case we need to fix more issues in PySide. Also, please note that I've not patched the python2 version pivy/interfaces/soqt2.i, yet. This still needs to be done. My posted fix should also work employing python 3 with win32, linux32 and linux64, however, I couldn't test this. Maybe you have the means to do that. If you are happy with the proposed solution and find it is working also on linux then I'd suggest that you may finally want to close this issue :-)

looooo commented 7 years ago

thanks for your collaboration. can you give me your e-mail address so I can add you as a contributor.

looooo commented 7 years ago

the pyside problem was patched in the conda-package: https://github.com/conda-forge/pyside-feedstock/commit/bd602466c3e31bc6e4689e5561eeb844d6d750cd

I think I will try to create pivy-packages for win and linux this weekend and look if the problems on win10 is still there.

@cgohlke do you have a list of patches you use for your pyside-package?

cgohlke commented 7 years ago

@cgohlke do you have a list of patches you use for your pyside-package?

Source code is at http://www.lfd.uci.edu/~gohlke/pythonlibs/#pyside in PySide-1.2.4-vc14-x64.zip. To fix the OverflowError, see the changes in conversions.h and sbkconverter_p.h.

looooo commented 7 years ago

thanks a lot @cgohlke . The diff of these files can be found here: https://github.com/looooo/pyside-feedstock/blob/master/recipe/win_pointer_conversation_fix.patch

looooo commented 7 years ago

finally this solves 10.2 on windows 10 and conda. That is a big step forward :-) so what is missing:

looooo commented 7 years ago

@InventorMentor I can add you as author of the mentioned commit later anyway. Just tell me how ou would like to handle this. I close this issue now as I think this example should work. At least I have tested with conda on: linux py34, py35, py36 win7, win8, win10