[Bug 14666] New: [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Summary: [wined3d] volume/volumetex locking implementation
           Product: Wine
           Version: CVS/GIT
          Platform: PC-x86-64
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: directx-d3d
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

Hi there,

this bug should be sort of a "metabug" for fixing the locking behaviour of
volume and volumetexture objects in D3D (version 8 and 9 currently, because 7
and below doesn't have/support volumes).

I already wrote some patches and testcases to fix/illustrate the problem, but
they were rejected on wine-patches. The main problem that the problem should be
adressed much lower in wined3d.

I now post all my current patches here, so other people can take a look.

Some things what are currently not implemented:
(1) locking of volume/volumetex objects always succeeds althoug it shouldn't
  - specifying to large lockboxes should fail for example
  - some volumes are not lockable at all
(2) creating volumes/volumetextures always succeeds, usage and pool types are
not checked

Only a few things the patches should adress.

I intend on working on this, but I need a bit more input about why and where to
fix this exactly.

Attaching patches now...
Also adding Stefan Dösinger, Henri Verbeet, Alexander Dorofeyev and Roderick
Colenbrander to CC (sry guys *g*, but I'm sure you know where my stuff should
go).

I currently have these DX SDK docs on my system:
version 9, 8, 7 and 6
(someone asked me if I got the 7 SDK)

Greets,
Tobias

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #1 from Tobias Jakobi   2008-07-27 17:59:58 ---
Created an attachment (id=15110)
 --> (http://bugs.winehq.org/attachment.cgi?id=15110)
[d3d8] validate pool type in CreateVolumeTexture

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #2 from Tobias Jakobi   2008-07-27 18:00:16 ---
Created an attachment (id=15111)
 --> (http://bugs.winehq.org/attachment.cgi?id=15111)
[d3d9] validate pool type in CreateVolumeTexture

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #3 from Tobias Jakobi   2008-07-27 18:00:34 ---
Created an attachment (id=15112)
 --> (http://bugs.winehq.org/attachment.cgi?id=15112)
[d3d8] handle invalid usage/pool combinations in CreateVolumeTexture

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #4 from Tobias Jakobi   2008-07-27 18:00:49 ---
Created an attachment (id=15113)
 --> (http://bugs.winehq.org/attachment.cgi?id=15113)
[d3d9] handle invalid usage/pool combinations in CreateVolumeTexture

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #5 from Tobias Jakobi   2008-07-27 18:01:08 ---
Created an attachment (id=15114)
 --> (http://bugs.winehq.org/attachment.cgi?id=15114)
[wined3d] correctly set lockable boolean in IWineD3DDeviceImpl_CreateVolume

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #6 from Tobias Jakobi   2008-07-27 18:01:28 ---
Created an attachment (id=15115)
 --> (http://bugs.winehq.org/attachment.cgi?id=15115)
[wined3d] honor lockable boolean in IWineD3DVolumeImpl_LockBox

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #7 from Tobias Jakobi   2008-07-27 18:01:55 ---
Created an attachment (id=15116)
 --> (http://bugs.winehq.org/attachment.cgi?id=15116)
[d3d8] add test for IDirect3DDevice8::CreateVolumeTexture (with locking)

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #8 from Tobias Jakobi   2008-07-27 18:02:10 ---
Created an attachment (id=15117)
 --> (http://bugs.winehq.org/attachment.cgi?id=15117)
[d3d9] add test for IDirect3DDevice9::CreateVolumeTexture (with locking)

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #9 from Tobias Jakobi   2008-07-27 18:02:49 ---
Created an attachment (id=15118)
 --> (http://bugs.winehq.org/attachment.cgi?id=15118)
[d3d9] locking test for volumes/volumetextures

unfinished

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #10 from Tobias Jakobi   2008-07-27 18:03:13 ---
Created an attachment (id=15119)
 --> (http://bugs.winehq.org/attachment.cgi?id=15119)
[d3d8] test locking for volumes/volumetextures

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Tobias Jakobi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Tobias Jakobi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Tobias Jakobi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Tobias Jakobi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #11 from Stefan Dösinger   2008-07-27 18:55:12 ---
I think this is better handled on wine-devel, but I am not sure about the
policies.

I'd recommend you to get the tests in first(with todo_wine markers for failing
tests). The tests look pretty good to me, just that you should use the return
value you got from D3D for error handling, not the expected value. E.g.

hr = Volume_Lock(vol);
ok(hr == expected, ...);
if(expected) { <--- Change this to hr
    unlock();
}

Once you have the volume tests in, you can port them over to textures and
cubetextures, and probably plain surfaces, and then port the texture and
cubetexture tests to ddraw to see how things work there. Once we know how ddraw
behaves we can make a more solid decision how to implement the behavior
properly in wine.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #12 from Tobias Jakobi   2008-07-28 04:19:56 ---
(In reply to comment #11)
> I think this is better handled on wine-devel, but I am not sure about the
> policies.

I posted it here to have a central place where to put unfinished patches and
ideas. Having this on wine-devel wasn't an option for me since things there get
a bit "deorganized" quickly :)
I hope the others don't mind if I post this here. If all of the locking stuff
is fixed I'm going to close this one.

Anyway, my primary motivation is a bunch of FIXME warnings from
IWineD3DVolumeImpl_LockBox (in wined3d/volume.c) that shows up when starting
Portal. When all is fixed I would like to have these removed :)
> I'd recommend you to get the tests in first(with todo_wine markers for failing
> tests). The tests look pretty good to me, just that you should use the return
> value you got from D3D for error handling, not the expected value. E.g.
>
> hr = Volume_Lock(vol);
> ok(hr == expected, ...);
> if(expected) { <--- Change this to hr
>     unlock();
> }

Sure, I use this approach then.
> Once you have the volume tests in, you can port them over to textures and
> cubetextures, and probably plain surfaces, and then port the texture and
> cubetexture tests to ddraw to see how things work there. Once we know how ddraw
> behaves we can make a more solid decision how to implement the behavior
> properly in wine.

OK, the reason I wanted to fix this first was that prefixing failing tests with
todo_wine was kinda impractical because of the BOOL array structures in my
test.
Maybe I drop them to get a cleaner test implementation.

So the plan is to finish/clean up the tests for volume creation and locking,
both for d3d8 and v9 and then doing similar stuff for tex and cubtex (also for
both d3d8 and v9, still have to check if <=7 has cubtex support), right?

If I wanna port everything that is supported over to ddraw, do I also have to
differentiate between all the older d3d versions? Like separate tests for
v1,2,3,4,5,6,7? Or can I build a generic test?

Cheers,
Tobias

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #13 from Stefan Dösinger   2008-07-28 08:50:20 ---
> So the plan is to finish/clean up the tests for volume creation and locking,
> both for d3d8 and v9 and then doing similar stuff for tex and cubtex (also for
> both d3d8 and v9, still have to check if <=7 has cubtex support), right?

Yes
> If I wanna port everything that is supported over to ddraw, do I also have to
> differentiate between all the older d3d versions? Like separate tests for
> v1,2,3,4,5,6,7? Or can I build a generic test?

I think just checking 7 in ddraw is ok for the start.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|CVS/GIT                     |unspecified

--- Comment #14 from Austin English   2009-01-20 02:39:39 ---
Removing deprecated CVS/GIT version tag. Please retest in current git. If the
bug is still present in today's wine, but was not present in some earlier
version of wine, please update version field to earliest known version of wine
that had the bug. Thanks!

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #15 from Stefan Dösinger   2009-01-20 04:26:28 ---
The volumetexture code didn't change in the past months, so I am pretty sure
this bug is still out there.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Tobias Jakobi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unspecified                 |1.1.13

--- Comment #16 from Tobias Jakobi   2009-01-24 17:39:54 ---
Yeah, Stefan is right.

However I didn't have enough time to work on the testcases.

I also have this bug (http://bugs.winehq.org/show_bug.cgi?id=14762) still open.
I think I'll fix this one first before coming back to the locking issues.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|1.1.13                      |unspecified

--- Comment #17 from Dmitry Timoshkov   2009-01-29 00:22:00 ---
1.1.13 doesn't belong to the date this bug has been reported.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #18 from Austin English   2009-07-30 12:11:52 ---
Is this still an issue in current (1.1.26 or newer) wine?

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #19 from Tobias Jakobi   2009-08-06 10:59:10 ---
Reconfirming with wine-1.1.26

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch, testcase

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

joaopa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #20 from joaopa  2011-05-29 00:45:52 CDT ---
still a bug in current git?

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #21 from Stefan Dösinger  2011-05-29 06:25:24 CDT ---
Yes, the bug still exists. Note that this isn't really a bug report, more a
TODO marker. The main point of this bug report is to store the patches, but
they are still stored if you close this bug.

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

--- Comment #22 from Austin English  2013-11-13 16:52:21 CST ---
This is your friendly reminder that there has been no bug activity for 2 years.
Is this still an issue in current (1.7.6 or newer) wine? If so, please attach
the terminal output in 1.7.6 (see http://wiki.winehq.org/FAQ#get_log).

[Bug 14666] [wined3d] volume/volumetex locking implementation

http://bugs.winehq.org/show_bug.cgi?id=14666

Stefan Dösinger  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #23 from Stefan Dösinger  2013-11-13 16:56:40 CST ---
I'd consider this bug fixed, by commits
a4f009d847671c5d5cd5fe07cfeb5348d75befb7..556e3e0c764ee5eff1eb02b97a01d4141d6ec58f
in dlls/wined3d/volume.c

[Bug 14666] [wined3d] volume/volumetex locking implementation

https://bugs.winehq.org/show_bug.cgi?id=14666

Bruno Jesus  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |556e3e0c764ee5eff1eb02b97a0
                   |                            |1d4141d6ec58f
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #24 from Bruno Jesus  ---
(In reply to Stefan Dösinger from comment #23)
> I'd consider this bug fixed, by commits
> a4f009d847671c5d5cd5fe07cfeb5348d75befb7..
> 556e3e0c764ee5eff1eb02b97a01d4141d6ec58f in dlls/wined3d/volume.c

Assuming fixed.

[Bug 14666] [wined3d] volume/volumetex locking implementation

https://bugs.winehq.org/show_bug.cgi?id=14666

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #25 from Alexandre Julliard  ---
Closing bugs fixed in 1.7.32.