http://bugs.winehq.org/show_bug.cgi?id=21515
--- Comment #72 from Stefan Dösinger stefandoesinger@gmx.at 2010-02-11 04:54:23 --- The patches look pretty good, just a few comments:
Patch 2: HW_VENDOR_MESA isn't needed any longer, just use HW_VENDOR_WINE if the real HW vendor isn't properly detected.
Patch 3:
- /* if (match_apple(gl_info, gl_renderer, gl_vendor, card_vendor, device)) return FALSE;
- if (strstr(gl_renderer, "DRI")) return FALSE; Filter out Mesa DRI drivers. */
return TRUE; Yes, shouldn't be necessary, just remove both lines. I'd also invert the gl_vendor if check if(GL_VENDOR == GL_VENDOR_ATI) return TRUE; else return FALSE;
Patch 4:
- for (i = 0; i < (sizeof(vendor_card_select_table) / sizeof(*vendor_card_select_table)); ++i)
- {
if ((vendor_card_select_table[i].gl_vendor != *gl_vendor)|| (vendor_card_select_table[i].card_vendor != *card_vendor))continue;TRACE_(d3d_caps)("Applying card_selector \"%s\".\n", vendor_card_select_table[i].description);return vendor_card_select_table[i].select_card(gl_info, gl_renderer, vidmem);- /* Default to generic Nvidia hardware based on the supported OpenGL extensions. The choice
I recommend to check the select_card result, to allow the card detection code to return an unknown card(e.g. 0x0000) if the detection fails for some reason. Then the loop can abort and use the generic nvidia card selection code below.
/* Geforce6/7 lowend */
/* If it's GL_VENDOR_APPLE, then it could also be an ATI card, so allow it to fall through */ *vidmem = 64; /* */ return CARD_NVIDIA_GEFORCE_6200; /* Geforce 6100/6150/6200/7300/7400/7500 */
Why would we end up in select_card_nvidia_binary on OSX with an ATI card?
Patch 5:
+enum wined3d_pci_device (select_card_intel_mesa)(const struct wined3d_gl_info *gl_info, const char *gl_renderer,
unsigned int *vidmem )+{
FIXME_(d3d_caps)("Card selection not handled for Mesa Intel driver\n");if (WINE_D3D9_CAPABLE(gl_info)) return CARD_NVIDIA_GEFORCEFX_5600;
This will lead to a PCI vendor 0x8086, pci device 0x0312 which doesn't exist. Some other nvidia card if interpreted as an Intel device might be a SATA Raid controller *gg*
That taps into the suggestion concerning patch 4 - allow the detection functions to fail, and add a FIXME to the generic nvidia guessing code, something like FIXME("Implement a card ID detection function for GL vendor %s, HW vendor %s\n", debug_gl_vendor(gl_vendor), debug_hw_vendor(hw_vendor));
General: I don't know how you generated the patches, but the patch format(filename, lack of author info etc) is unknown to me. I recommend to use git-format-patch, it allows you to add a description of what the patch does into the patch, and adds your name and Email address. There are some higher level tools like stacked git that can do this as well.