On 28 October 2015 at 15:29, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
Version 2: Don't use fbo_entry->d3d_* for logging, plan for it to go away. The disadvantage is now that we don't know the d3d properties once we find out that an FBO doesn't work and need a full TRACE log for this information, but I think that's something we can live with.
If there is a prettier way to find the texture target of a GL texture name please advise. I couldn't find any.
I think I'll hold off on this until the regression from 1ca9dfc8ee25f4ae188fdacd4d3d56046cef8003 is resolved, but you can probably split the logging changes into their own patch.
+ char name[32]; + sprintf(name, "Color attachment %u", i); + context_dump_fbo_attachment(gl_info, target, GL_COLOR_ATTACHMENT0 + i, name); sprintf() is one of those things that's usually best avoided. In this case the string is mostly redundant as well, since you have the attachment enum.
+static inline void context_create_fbo_key(struct wined3d_fbo_entry_key *key, + struct wined3d_surface **render_targets, struct wined3d_surface *depth_stencil, + DWORD color_location, DWORD ds_location, UINT color_buffers) The naming is a bit odd. "create" is usually for things that allocate something, "color_buffers" would normally be called e.g. "rt_count". The "inline" seems speculative. I think modern compilers mostly ignore it, but it does make it harder to notice when a function becomes unused.
- entry = HeapAlloc(GetProcessHeap(), 0, sizeof(*entry)); - entry->render_targets = HeapAlloc(GetProcessHeap(), 0, gl_info->limits.buffers * sizeof(*entry->render_targets)); - memcpy(entry->render_targets, render_targets, gl_info->limits.buffers * sizeof(*entry->render_targets)); - entry->depth_stencil = depth_stencil; - entry->color_location = color_location; - entry->ds_location = ds_location; + UINT object_count = gl_info->limits.buffers + 1; + + entry = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, + FIELD_OFFSET(struct fbo_entry, key.objects[object_count])); + entry->d3d_render_targets = HeapAlloc(GetProcessHeap(), 0, + gl_info->limits.buffers * sizeof(*entry->d3d_render_targets)); + context_create_fbo_key(&entry->key, render_targets, depth_stencil, color_location, ds_location, + gl_info->limits.buffers); + memcpy(entry->d3d_render_targets, render_targets, sizeof(*entry->d3d_render_targets) * gl_info->limits.buffers); + entry->d3d_depth_stencil = depth_stencil; Why HEAP_ZERO_MEMORY? Do we no longer initialize all fields?
+ { + struct wined3d_fbo_resource resource = {0}; + context_attach_depth_stencil_fbo(context, target, &resource, FALSE, 0, 0); "resource" should probably be static const.
- if (surface == entry->render_targets[i]) + if (surface->container->texture_rgb.name == entry->key.objects[i].object || + surface->container->texture_srgb.name == entry->key.objects[i].object) This is different from the usual style.