On 27/05/07, Phil Costin philcostin@hotmail.com wrote:
Hi,
This patch has been a long time in the making. The following test was used during the creation of the patch:
If it's not too much trouble, a proper wine test would be nice.
The changes to surface_upload_data and surface_download_data look like they should be separate changes.
In cubetexture.c, and similar code in texture.c and volume.c:
- } else if (GL_SUPPORT(EXT_TEXTURE_SRGB) && This->baseTexture.bindCount > 0) {
currentSrgbMode = This->resource.wineD3DDevice->stateBlock->samplerState[This->baseTexture.sampler][WINED3DSAMP_SRGBTEXTURE];
srgbModeChanged = (This->baseTexture.srgbWhenLoaded != currentSrgbMode);
This->baseTexture.srgbWhenLoaded = currentSrgbMode;
You've got "device" here, no need to use "This->resource.wineD3DDevice"
sRGBmode looks a bit awkward as a variable name, I'd rather see something like srgb_mode, but I guess that's more a personal preference.
It would also probably be better to submit/commit this right after a release instead of right before one.
Appart of two things Henri already said the patch looks good to me(Use device in texture.c, break out the changes to surface_upload_data and surface_download data into seperate patches).
As for a wine test, I think its not easy to test it reliably since the opengl extension gives implementors quite a lot of room and doesn't require exact results. I think all we can reasonably test for are the oddnessnes hl2 requires, like gamma-corrected Q8W8V8U8 and what the rendering outcome is - but that should go into a different patch.