On Tue, Jan 12, 2010 at 3:18 PM, Luca Barbieri <l...@luca-barbieri.com> wrote: >> Can we keep the logic as simple as: >> if (not forced) { >> get_current_sequence_number(); >> if (seq_num has not changed) >> continue; >> } >> validate_and_update_buffers(); >> There should be no need to reset the sequence number or comparing the >> surface. > Resetting the sequence number on surface change is necessary because > the sequence number is relative to the surface (if all surface changes > set forced validation, this may be removed though). > It is also used for forced validation. > To get the current sequence number, you need to make a DRI2 GetBuffers call. > Your logic could be implemented by splitting validate into a > ->validate_and_get_sequence_number member of native_surface that would > only get the names, and destroy old textures if outdated, and another > that actually gets the surfaces. > Not sure whether the current design or that is better. Semantically, the validate call asks for nothing but the specified textures of the native surface _at the moment_. There are some properties
* It can be called repeatedly without any ill effect * It may return entirely different textures between two calls, without any knowledge in advance As such, splitting the call into "get sequence number" and "validate" is not possible. For optimization, some artifacts are added (and documented) * The argument "textures" may be NULL. Since there will be no way to return the textures to the caller, the native display may skip the creation of the textures. * (NEW) A sequence number is returned to indicate which moment the textures belong to (even when the argument "textures" is NULL) Using these artifacts, EGL is capable to * validate the native surface with "textures" set to NULL to get the sequence number * if the sequence number differs from what EGLSurface knows (which rarely happens), validate is called again to retrieve the textures The "force_validate" is used to indicate that the first validate call is very likely to return a different sequence number, and it may call to retrieve the textures directly. > Another option is to make the textures[] array, the sequence number, > and width/height public members of native_surface, and have a single > validate function that would update this data, leaving to the caller > the task of referencing the texture if he wants. > Since both DRI2 and ximage already store this data in their own > private surface structure, this may be the simplest design and would > actually reduce the amount of code in both the backends and the EGL > code. I prefer not to export these members because they are _always_ outdated logically. >> Could we make it: >> >> /** >> * Validate the buffers of the surface. The returned textures are owned by >> * the caller. A sequence number is also returned. The caller can use it >> * to see if anything has changed since the last call. Any of the pointers >> * may be NULL and it indicates the caller has no interest in those values. >> * >> * If this function is called multiple times with different attachments, >> * those not listed in the latest call might be destroyed. This behavior >> * might change in the future. >> */ >> >> That is, seq_num is no longer an input. > That's possible, but it will add the work of increasing the reference > number of all returned textures in the validate call, and then > decreasing it in egl_g3d.c when it realizes that the sequence number > did not change. The native display needs not to create the textures when the caller does not ask to return them. >>> - dri2surf->have_back = FALSE; >>> - dri2surf->have_fake = FALSE; >>> - >>> xbufs = x11_drawable_get_buffers(dri2dpy->xscr, dri2surf->drawable, >>> - &dri2surf->width, &dri2surf->height, >>> + &templ.width0, &templ.height0, >>> dri2atts, FALSE, num_ins, &num_outs); >>> if (!xbufs) >>> return FALSE; >>> >>> - /* update width and height */ >>> - templ.width0 = dri2surf->width; >>> - templ.height0 = dri2surf->height; >>> + if(!dri2surf->seq_num || dri2surf->width != templ.width0 || >>> dri2surf->height != templ.height0) >>> + { >>> + /* update width and height */ >>> + dri2surf->width = templ.width0; >>> + dri2surf->height = templ.height0; >>> + >>> + if(!++dri2surf->seq_num) >>> + ++dri2surf->seq_num; /* wrap to 1, not 0 */ >>> + } >>> + else if(seq_num && *seq_num == dri2surf->seq_num) >>> + goto out; >> If this function is called twice, one with a back attachement followed by one >> without a back attachment, x11_drawable_get_buffers above will destroy the >> back >> buffer in the second call. We need to update dri2surf->have_back >> before bailing out. > You are right. > That shouldn't happen though, because a call with the latest sequence > number must have the same attachments, since changing attachments must > be done with a forced revalidation. > This should be either modified to look at the returned attachments > before bailing out or the requirement of not changing attachments > without zeroing out the sequence numbers should be documented. ------------------------------------------------------------------------------ This SF.Net email is sponsored by the Verizon Developer Community Take advantage of Verizon's best-in-class app development support A streamlined, 14 day to market process makes app distribution fast and easy Join now and get one step closer to millions of Verizon customers http://p.sf.net/sfu/verizon-dev2dev _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev