Re: [PATCH] d3drm/tests: Add tests for IDirect3DRM*::LoadTexture (try 4).
Am 23.12.2015 um 16:46 schrieb Aaryaman Vasishta <jem456.vasishta(a)gmail.com>:
+ file = create_bitmap("palette.bmp", 3, 39, TRUE); + + hr = Direct3DRMCreate(&d3drm1); + ok(hr == D3DRM_OK, "Cannot get IDirect3DRM interface (hr = %x)\n", hr); + + hr = IDirect3DRM_QueryInterface(d3drm1, &IID_IDirect3DRM2, (void **)&d3drm2); + ok(SUCCEEDED(hr), "Cannot get IDirect3DRM2 interface (hr = %x)\n", hr); + + file = create_bitmap("8bpp.bmp", 100, 100, FALSE); The first line here seems to be a leftover from the last patch version. The same applies to the V3 function
On Sat, Dec 26, 2015 at 11:46 PM, Stefan Dösinger <stefandoesinger(a)gmail.com
wrote:
Am 23.12.2015 um 16:46 schrieb Aaryaman Vasishta < jem456.vasishta(a)gmail.com>:
+ file = create_bitmap("palette.bmp", 3, 39, TRUE); + + hr = Direct3DRMCreate(&d3drm1); + ok(hr == D3DRM_OK, "Cannot get IDirect3DRM interface (hr = %x)\n", hr); + + hr = IDirect3DRM_QueryInterface(d3drm1, &IID_IDirect3DRM2, (void **)&d3drm2); + ok(SUCCEEDED(hr), "Cannot get IDirect3DRM2 interface (hr = %x)\n", hr); + + file = create_bitmap("8bpp.bmp", 100, 100, FALSE); The first line here seems to be a leftover from the last patch version. The same applies to the V3 function
My bad, that line is definitely redundant. Anything else before I send try 5? Hopefully that'll be the last one :) Cheers, Aaryaman
Am 26.12.2015 um 19:51 schrieb Aaryaman Vasishta <jem456.vasishta(a)gmail.com>:
Anything else before I send try 5? Hopefully that'll be the last one :) I'm sitting in a crowded train with crappy internet access right now, so reviews are a bit tricky :-) .
Actually I noticed quite a lot on a second look:
+ info = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(BITMAPINFO)); I prefer sizeof(*info) instead of sizeof(BITMAPINFO) in this case, but it's mostly a matter of style.
+ WriteFile(file, (void *)info, size, &written, NULL); A cast from / to void * is not necessary in C. (It's a C++ thingy).
You could add a few more checks for bpp and bytes_per_line in test_bitmap_data(). Most of those would be inside the format-specific paths.
+ ok(size != INVALID_FILE_SIZE, "Failed to get file size, error %d\n", GetLastError()); + if (size == INVALID_FILE_SIZE) + goto cleanup; This is another case where you can skip the ifs, if the branch is taken the test already failed anyway.
+ buffer2 = ((unsigned char *)buffer + bmp_header->bfOffBits); The outer parenthesis are kinda redundant
+ file = create_bitmap("8bpp.bmp", 100, 100, FALSE); ... + file = create_bitmap("24bpp.bmp", 100, 100, FALSE); ... I don't think those two differ. I guess the first one should have palletized = TRUE
I hope that's all now, but I can't promise...
On Sun, Dec 27, 2015 at 12:40 AM, Stefan Dösinger <stefandoesinger(a)gmail.com
wrote:
Am 26.12.2015 um 19:51 schrieb Aaryaman Vasishta < jem456.vasishta(a)gmail.com>:
Anything else before I send try 5? Hopefully that'll be the last one :) I'm sitting in a crowded train with crappy internet access right now, so reviews are a bit tricky :-) .
I hope that's all now, but I can't promise...
No problem. Thanks for the review! I'll make the changes. Do reply later on if there's more. I'll send a try 5 tomorrow.
On Sun, Dec 27, 2015 at 12:40 AM, Stefan Dösinger <stefandoesinger(a)gmail.com
wrote:
You could add a few more checks for bpp and bytes_per_line in test_bitmap_data(). Most of those would be inside the format-specific paths.
I've sent try 5 with some more tests and also added odd/even width tests for palettized textures, and format-specific depth/bpp tests along with the rest of the changes. Cheers, Aaryaman
participants (2)
-
Aaryaman Vasishta -
Stefan Dösinger