kmonsoor / pyglet

Automatically exported from code.google.com/p/pyglet
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

bug in font/win32query.py on win x64 (its not always occur ) #664

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Run examples/html_label.py
2. Run it again about 10 times.
3. The exception must be raised:

  File "XXX\pyglet\pyglet\font\win32query.py", line 339, in query
    0)
ctypes.ArgumentError: argument 1: <type 'exceptions.OverflowError'>: long int 
too long to convert

sys.version: 2.7.5 (default, May 15 2013, 22:44:16) [MSC v.1500 64 bit (AMD64)]
sys.platform: win32
sys.maxint: 2147483647

pyglet.version: 1.2alpha1

Original issue reported on code.google.com by falsedr...@gmail.com on 23 Sep 2013 at 12:31

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
This problem is due to a sign extension problem in the ctypes wrapper for 
GetDC().

The Win32 function returns a 32bit value, and it’s (improperly) getting sign 
extended to 64bits somehow by ctypes.  About half the time, the handle returned 
by this function will have the high bit set, which is why the bug is only 
sometimes reproducible.

In [1]: from ctypes import *

In [10]: a = windll.user32.GetDC(None)

In [11]: a
Out[11]: -2013190841

In [23]: type(a)
Out[23]: int

In [24]: hex(a)
Out[24]: '-0x77fedab9'

I’ve tried a bunch of likely hacks in Pyglet to fix this, but nothing has 
worked.  Here’s a good clue:

http://stackoverflow.com/questions/3323778/is-pythons-ctypes-c-long-64-bit-on-64
-bit-systems

We've successfully hacked around this by masking the return values from GetDC() 
as in:

pyglet\window\win32\__init__.py, line 224:

self._dc = _user32.GetDC(self._view_hwnd) & 0x00000000FFFFFFFF

Original comment by jaybors...@gmail.com on 6 Feb 2014 at 8:44

GoogleCodeExporter commented 9 years ago

Original comment by useboxnet on 8 Feb 2014 at 2:37

GoogleCodeExporter commented 9 years ago
Sure this deserves a reference to some issue on bugs.python.org because I'd say 
this problem is pretty major and should be fixed on Python level at least in 
docs.

Original comment by techtonik@gmail.com on 9 Feb 2014 at 10:52

GoogleCodeExporter commented 9 years ago
Here's a stackoverflow question on the issue:  
http://stackoverflow.com/questions/21686360/why-does-python-2-7-sign-extend-hdc-
returned-from-getdc-on-x64 

So, Lib/ctypes/wintypes.py defines:

HANDLE = ctypes.c_void_p # in the header files: void *
HDC = HANDLE

But doesn't have the code to actually implement GetDC() which is in
pyglet/ libs/ win32/ __init__.py 

So I can't really say whose bug this is... but it's broken!

Original comment by jaybors...@gmail.com on 10 Feb 2014 at 10:15

GoogleCodeExporter commented 9 years ago
Here's a potential solution:

http://stackoverflow.com/questions/17840144/why-does-setting-ctypes-dll-function
-restype-c-void-p-return-long  

Adding the following:
class c_void_p(c_void_p): 
    pass

to the top of pyglet\libs\win32\__init__.py fixed the issue for me!

Original comment by jaybors...@gmail.com on 11 Feb 2014 at 7:25

GoogleCodeExporter commented 9 years ago
Sorry to recant, but adding: 

class c_void_p(c_void_p): 
    pass

to the top of pyglet\libs\win32\__init__.py does NOT fix the issue.  I 
apparently didn't test enough before posting.

Original comment by jaybors...@gmail.com on 11 Feb 2014 at 11:01

GoogleCodeExporter commented 9 years ago

Original comment by useboxnet on 13 Feb 2014 at 6:32

GoogleCodeExporter commented 9 years ago
Here's an implementation which seems to work:

https://github.com/animehunter/SublimeColorPickerWindowsOnly/blob/master/colorpi
cker_command.py 

    GetDC = ctypes.windll.User32.GetDC
    GetDC.argtypes = [c_void_p]
    GetDC.restype = c_void_p

    ReleaseDC = ctypes.windll.User32.ReleaseDC
    ReleaseDC.argtypes = [c_void_p, c_void_p] #hwnd, hdc
    ReleaseDC.restype = c_int32

Original comment by jaybors...@gmail.com on 6 May 2014 at 11:12

GoogleCodeExporter commented 9 years ago
I've attached a patch with jayborseth proposal from #9; can you all give it a 
go to see if it works?

I'd love to commit the change but I can't test it.

Original comment by useboxnet on 19 May 2014 at 4:43

Attachments:

GoogleCodeExporter commented 9 years ago
Can you write a test that fails, so that it can be tested automatically?

Original comment by techtonik@gmail.com on 23 May 2014 at 8:15

GoogleCodeExporter commented 9 years ago
I don't have a windows machine to test it, I need help on that.

Original comment by useboxnet on 23 May 2014 at 8:17

GoogleCodeExporter commented 9 years ago

Original comment by useboxnet on 30 May 2014 at 6:09

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 021505b88a84.

Original comment by useboxnet on 31 May 2014 at 7:53

GoogleCodeExporter commented 9 years ago
I can include tests for this if anyone can provide them.

Original comment by useboxnet on 31 May 2014 at 7:54