Hi Tomasz,
first of all, thanks for sending this patch! Much appreciated. Comments
below.
On Fri, 29 Oct 2010 19:20:17 +0200, Tomasz Rybak bogom...@post.pl wrote:
The biggest problem is that IMO (I might be wrong - do
not know PyCUDA to such intimate details) it introduces
non-backward-compatible change.
Why do this? Why not introduce a new interface and leave the old one in
place? That way, we can compile both interfaces as long as they're
available, and it's up to the user which one to use. We shouldn't be in
the business of imposing API use restrictions on PyCUDA users.
I have split BufferObject into two:
BufferObject responsible for buffers (VBO, etc.)
ImageObject responsible for textures, etc.
Previously BufferObject was responsible for both those
cases - now anyone who wants to use CUDA on images
must use ImageObject (which is API-breaking change).
Can you explain the reasoning for this split? A brief glance over the
code didn't seem to reveal many differences. If nothing else, they
should probably inherit from a common base that absorbs the shared code.
Also, I noticed that your use of cudaGrahpicsXXX flags with
cuGrahpicsGLRegisterBuffer() is likely erroneous, as is visible in this
bit of documentation. There are equivalent CU_... flags which we should
use instead. See docs here (or in the header):
http://developer.download.nvidia.com/compute/cuda/3_0-Beta1/toolkit/docs/online/group__CUGL_ge148ed92fe0728be19c9281302b5922c.html
Also fishy: image_object, if it is different code, should probably be
using cuGraphicsGLRegisterImage() (rather than ... Buffer()).
I also attach problem sample code (Qt+OpenGL+PyCUDA)
that uses new BufferObject.
I have checked and it works after applying my patch;
I only had to change initialisation to get program
using VBO running after applying my patch.
I still don't like that we need to change the init sequence--why is this
necessary?
I have not checked ImageObject.
I have also started writing documentation for new pycuda.gl
functions.
I have removed gl_init as documentation says that cuGLInit()
is deprecated and is not doing anything (Reference 4.41.4.1).
Again, as above, we shouldn't be making API use decisions for our
users. I assume it did do something at some point in the past, so as
long as feasible we should still wrap it.
Instead I have created gl_select_device; this function however
must be called as the first (before all other CUDA functions),
is cuda*, not cu* function, does not cooperate with
CUDAPP_CALL_GUARDED - so I do not know what to do with it.
I was not able to test it - I do not have setup with two devices.
As above, I don't like mixing in cuda* (run-time API) functionality with
cu* (driver API) functionality.
Other problems:
Function pycuda.tools.get_default_device is depreciated,
but pycuda.gl.autoinit uses it. I have changed autoinit
to use Device(0), but it is not elegant and should be changed.
Function pycuda.tools.make_default_context contains
repeated code checking for presence of CUDA device:
def make_default_context():
ndevices = cuda.Device.count()
if ndevices == 0:
errmsg = No CUDA enabled device found. Please check your
installation.
raise RuntimeError, errmsg
ndevices = cuda.Device.count()
if ndevices == 0:
raise RuntimeError(No CUDA enabled device found.
Please check your installation.)
Why? Is that omission, or does this double checking serve
some purpose?
Whoops. Fixed.
Andreas
pgpWcF3ZFrtNT.pgp
Description: PGP signature
___
PyCUDA mailing list
PyCUDA@tiker.net
http://lists.tiker.net/listinfo/pycuda
