>> - num_surfaces = (gctx->base.ReadSurface == gctx->base.DrawSurface) ? 1 :
>> 2;
>> - for (s = 0; s < num_surfaces; s++) {
>> + for (s = 0; s < 2; s++) {
> Why this change?
Ignore it.
>> struct pipe_texture *textures[NUM_NATIVE_ATTACHMENTS];
>> struct egl_g3d_surface *gsurf;
>> struct egl_g3d_buffer *gbuf;
>> + int cur_seq_num;
>>
>> if (s == 0) {
>> gsurf = egl_g3d_surface(gctx->base.DrawSurface);
>> @@ -62,24 +62,20 @@ egl_g3d_validate_context(_EGLDisplay *dpy, _EGLContext
>> *ctx)
>> gbuf = &gctx->read;
>> }
>>
>> - if (!gctx->force_validate) {
>> - EGLint cur_w, cur_h;
>> -
>> - cur_w = gsurf->base.Width;
>> - cur_h = gsurf->base.Height;
>> - gsurf->native->validate(gsurf->native,
>> - gbuf->native_atts, gbuf->num_atts,
>> - NULL,
>> - &gsurf->base.Width, &gsurf->base.Height);
>> - /* validate only when the geometry changed */
>> - if (gsurf->base.Width == cur_w && gsurf->base.Height == cur_h)
>> - continue;
>> - }
>> + if (gctx->force_validate || gsurf != gbuf->surface)
>> + gbuf->seq_num = 0;
>> + cur_seq_num = gbuf->seq_num;
>>
>> gsurf->native->validate(gsurf->native,
>> gbuf->native_atts, gbuf->num_atts,
>> (struct pipe_texture **) textures,
>> - &gsurf->base.Width, &gsurf->base.Height);
>> + &gsurf->base.Width, &gsurf->base.Height, &gbuf->seq_num);
>> +
>> + if(gbuf->seq_num == cur_seq_num)
>> + continue;
>> +
>> + gbuf->surface = gsurf;
>> +
> 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.
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.
> 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.
>> */
>> boolean (*validate)(struct native_surface *nsurf,
>> const enum native_attachment *natts,
>> unsigned num_natts,
>> struct pipe_texture **textures,
>> - int *width, int *height);
>> + int *width, int *height, int* seq_num);
>>
>> /**
>> * Wait until all native commands affecting the surface has been executed.
>> diff --git a/src/gallium/state_trackers/egl_g3d/x11/native_dri2.c
>> b/src/gallium/state_trackers/egl_g3d/x11/native_dri2.c
>> index 890e11d..e38695b 100644
>> --- a/src/gallium/state_trackers/egl_g3d/x11/native_dri2.c
>> +++ b/src/gallium/state_trackers/egl_g3d/x11/native_dri2.c
>> @@ -67,6 +67,7 @@ struct dri2_surface {
>> unsigned names[NUM_NATIVE_ATTACHMENTS];
>> boolean have_back, have_fake;
>> int width, height;
>> + int seq_num;
>> };
>>
>> struct dri2_config {
>> @@ -140,7 +141,7 @@ dri2_surface_validate(struct native_surface *nsurf,
>> const enum native_attachment *natts,
>> unsigned num_natts,
>> struct pipe_texture **textures,
>> - int *width, int *height)
>> + int *width, int *height, int* seq_num)
>> {
>> struct dri2_surface *dri2surf = dri2_surface(nsurf);
>> struct dri2_display *dri2dpy = dri2surf->dri2dpy;
>> @@ -211,18 +212,26 @@ dri2_surface_validate(struct native_surface *nsurf,
>> texture_indices[natts[i]] = i;
>> }
>>
>> - 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
