On 23 March 2016 at 20:05, Aaryaman Vasishta jem456.vasishta@gmail.com wrote:
On windows, IDirect3DRMTexture*::Load creates a new texture every time you call it on the same texture pointer, effectively losing the old one and creating a leak. The refcount also increases by one every time a new texture is created. So if windows allows this, I guess it's fine. Do let me know if I've missed testing something here though.
So the image can't be replaced after it has been set.
AFAICS on the trace logs of d3drm games ran so far, I haven't encountered CreateTexture spefically. Though you can find usage of CreateTexture in the "Trans" demo from the DX7 SDK, within trans.cpp. After a quick glance I can confirm that it does indeed create a texture on an existing image in memory. The implementation for this should be fairly trivial, but some tests would be needed to check if it does a shallow/deep copy of the image struct (as you mentioned), validation checks, if any, and its refcounting behavior.
I'm not sure about the Init methods, but so far I haven't been able to see them being used anywhere.
I've initially decided to prioritize on functions that are actually used by games rather than implementing functions that are probably not used at all in the d3drm apps that ran on wine so far. But I don't mind doing them if they're required somehow, or if a bug reports it.
Sure, but it's useful to have some idea what those functions are supposed to do. Right now, I think it looks like you should be able to implement LoadTexture() by creating a texture and then calling InitFromFile() on it. And related to above, InitFromFile() should then probably check that the texture hasn't been initialised before.
Right, I could use it to check if the size matches the size specifed in the header field bfSize within BITMAPFILEHEADER. Or maybe there's another use than this?
Mostly to make sure you don't read past the end of the buffer.
+D3DRMIMAGE *d3drm_create_image(unsigned char *buffer, BITMAPINFO *info, BOOL palette, BOOL upside_down) +{
- D3DRMPALETTEENTRY *colors = (D3DRMPALETTEENTRY
*)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, 257 * sizeof(*colors));
Is "257" there intentional? The cast is unnecessary. HeapAlloc() can fail. Do you need HEAP_ZERO_MEMORY here?
I kept it as 257 so that accessing colors[256] shouldn't segfault. The approach I used here is to check if the total number of unique colors is greater than 256, which happens when the 257th unique color is also stored into the array.
It's probably better to just avoid writing it.
I guess statically allocating it on the stack i.e declaring D3DRMPALETTEENTRY colors[257]; could possibly help avoid the HeapAlloc failures. Makes sense since the 257 is hard-coded anyways. Should I do that? Or is the HeapAlloc a better approach?
HeapAlloc() is fine, you just need to handle the failure.