[virglrenderer-devel] [PATCH 2/3] vrend: If available use glCopyImageSubData to execute memcopy like blits
Gert Wollny
gert.wollny at collabora.com
Tue Jun 26 07:10:16 UTC 2018
Am Montag, den 25.06.2018, 20:52 -0700 schrieb Gurchetan Singh:
> On Wed, Jun 20, 2018 at 2:03 AM Gert Wollny <gert.wollny at collabora.co
> m> wrote:
> >
> > When the host is gles >= 3.2, gl >= 4.3 or when the extension
> > GL_(ARB|EXT|OES)_copy_image is available, memcopy like blitting and
> > region
> > copying can be done for many color format combinations by using
> > glCopyImageSubData.
> >
> > This fixes a number of tests from the subset
> > dEQP-GLES31.functional.copy_image.non_compressed.viewclass_*
> >
> > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > ---
> > src/vrend_formats.c | 25 ++++++++++++++++++++++
> > src/vrend_renderer.c | 58
> > ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > src/vrend_renderer.h | 2 ++
> > 3 files changed, 83 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/vrend_formats.c b/src/vrend_formats.c
> > index eb9f217..d1653a4 100644
> > --- a/src/vrend_formats.c
> > +++ b/src/vrend_formats.c
> > @@ -443,3 +443,28 @@ void vrend_build_format_list_gles(void)
> > */
> > add_formats(gles_z32_format);
> > }
> > +
> > +bool vrend_is_canonical_format(enum pipe_format format)
> > +{
> > + /* These are the canonical formats that are set by gallium */
> > +
> > + switch (format) {
> > + case PIPE_FORMAT_R32G32B32A32_UINT:
> > + case PIPE_FORMAT_R32G32B32_UINT:
> > + case PIPE_FORMAT_R32G32_UINT:
> > + case PIPE_FORMAT_R16G16B16A16_UINT:
> > + case PIPE_FORMAT_R16G16B16_UINT:
> > + case PIPE_FORMAT_R8G8B8A8_UNORM:
> > + case PIPE_FORMAT_A8B8G8R8_UNORM:
> > + case PIPE_FORMAT_R16G16_UNORM:
> > + case PIPE_FORMAT_R32_UINT:
> > + case PIPE_FORMAT_R8G8B8_UINT:
> > + case PIPE_FORMAT_G8R8_UNORM:
> > + case PIPE_FORMAT_R8G8_UNORM:
> > + case PIPE_FORMAT_R16_UINT:
> > + case PIPE_FORMAT_R8_UINT:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
>
> What do you mean by "canonical" formats? st_cb_copyimage.c makes
> some references to canonical formats, but doesn't really list it like
> this as this patch does.
In st_cb_copyimage.c get_canonocal_format the various format names are
mapped to these PIPE_FORMAT_* types, here I check whether the the
current format is one of those that is returned by this function,
that's why I thought the name would be apropriated.
>
>
> > \ No newline at end of file
> > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > index b769e8a..8690eb2 100644
> > --- a/src/vrend_renderer.c
> > +++ b/src/vrend_renderer.c
> > @@ -118,6 +118,7 @@ struct global_renderer_state {
> > bool have_polygon_offset_clamp;
> > bool have_texture_storage;
> > bool have_tessellation;
> > + bool have_copy_image;
> >
> > /* these appeared broken on at least one driver */
> > bool use_explicit_locations;
> > @@ -4449,6 +4450,12 @@ int vrend_renderer_init(struct vrend_if_cbs
> > *cbs, uint32_t flags)
> > if (gl_ver >= 46 ||
> > epoxy_has_gl_extension("GL_ARB_polygon_offset_clamp"))
> > vrend_state.have_polygon_offset_clamp = true;
> >
> > + if (gl_ver >= 43 || (gles && gl_ver >= 32) ||
> > + epoxy_has_gl_extension("GL_ARB_copy_image") ||
> > + epoxy_has_gl_extension("GL_EXT_copy_image") ||
> > + epoxy_has_gl_extension("GL_OES_copy_image"))
> > + vrend_state.have_copy_image = true;
> > +
> > /* callbacks for when we are cleaning up the object table */
> > vrend_resource_set_destroy_callback(vrend_destroy_resource_obje
> > ct);
> > vrend_object_set_destroy_callback(VIRGL_OBJECT_QUERY,
> > vrend_destroy_query_object);
> > @@ -6260,6 +6269,22 @@ static void
> > vrend_resource_copy_fallback(struct vrend_context *ctx,
> > free(tptr);
> > }
> >
> > +
> > +static inline void
> > +vrend_copy_sub_image(struct vrend_resource* src_res, struct
> > vrend_resource * dst_res,
> > + uint32_t src_level, const struct pipe_box
> > *src_box,
> > + uint32_t dst_level, uint32_t dstx, uint32_t
> > dsty, uint32_t dstz)
> > +{
> > + glCopyImageSubData(src_res->id,
> > + tgsitargettogltarget(src_res->base.target,
> > src_res->base.nr_samples),
> > + src_level, src_box->x, src_box->y, src_box-
> > >z,
> > + dst_res->id,
> > + tgsitargettogltarget(dst_res->base.target,
> > dst_res->base.nr_samples),
> > + dst_level, dstx, dsty, dstz,
> > + src_box->width, src_box->height,src_box-
> > >depth);
> > +}
> > +
> > +
> > void vrend_renderer_resource_copy_region(struct vrend_context
> > *ctx,
> > uint32_t dst_handle,
> > uint32_t dst_level,
> > uint32_t dstx, uint32_t
> > dsty, uint32_t dstz,
> > @@ -6292,11 +6317,20 @@ void
> > vrend_renderer_resource_copy_region(struct vrend_context *ctx,
> > return;
> > }
> >
> > + if (vrend_state.have_copy_image) {
> > + const struct util_format_description *src_desc =
> > util_format_description(src_res->base.format);
> > + const struct util_format_description *dst_desc =
> > util_format_description(dst_res->base.format);
> > + if (util_is_format_compatible(src_desc,dst_desc)) {
> > + vrend_copy_sub_image(src_res, dst_res, src_level,
> > src_box,
> > + dst_level, dstx, dsty, dstz);
> > + return;
> > + }
> > + }
> > +
> > if (!vrend_format_can_render(src_res->base.format) ||
> > !vrend_format_can_render(dst_res->base.format)) {
> > vrend_resource_copy_fallback(ctx, src_res, dst_res,
> > dst_level, dstx,
> > dsty, dstz, src_level,
> > src_box);
> > -
> > return;
> > }
> >
> > @@ -6568,7 +6602,27 @@ void vrend_renderer_blit(struct
> > vrend_context *ctx,
> > if (info->render_condition_enable == false)
> > vrend_pause_render_condition(ctx, true);
> >
> > - vrend_renderer_blit_int(ctx, src_res, dst_res, info);
> > + /* The gallium blit function can be called for a general blit
> > that may
> > + * scale and convert the data or for a memcopy blit. In the
> > latter case
> > + * the info->formats are equal and set to one of the canonical
> > formats.
> > + * In addition some other restrictions apply. For this latter
> > case use the
> > + * glCopyImageSubData function.
> > + */
> > + if (vrend_state.have_copy_image &&
> > + vrend_is_canonical_format(info->src.format) &&
> > + (info->src.format == info->dst.format) &&
> > + !info->scissor_enable && (info->filter ==
> > PIPE_TEX_FILTER_NEAREST) &&
> > + !info->alpha_blend && (info->mask == PIPE_MASK_RGBA) &&
> > + (src_res->base.nr_samples == dst_res->base.nr_samples) &&
> > + info->src.box.width == info->dst.box.width &&
> > + info->src.box.height == info->dst.box.height &&
> > + info->src.box.depth == info->dst.box.depth) {
> > + vrend_copy_sub_image(src_res, dst_res, info->src.level,
> > &info->src.box,
> > + info->dst.level, info->dst.box.x, info-
> > >dst.box.y,
> > + info->dst.box.z);
> > + } else {
> > + vrend_renderer_blit_int(ctx, src_res, dst_res, info);
> > + }
>
> Hmm, do the GLES31 tests fail when doing blit framebuffer or during
> the GL fallback?
The framebuffer blits usually fail because the formats are not
compatible (for a blit source and target both either have to be float,
or, if they are integer, then they have to be of the same signedness).
Also, a blit does _convert_, e.g. a R16 format copied onto a R8G8
format will only fill the red destination component and set the green
component to zero. However, copy_image is supposed to do the equivalent
of a memcopy.
In the GL path one could probably fix this with the right shaders, but
there one still would have to deal with sRGB <-> RGB copies, because
sRGB is normally converted by the texturing hardware (if I understood
this right), and a shader would either have to undo this, or emulate it
(depending on the copy direction).
> Since this requires GLES32 features, but an important use case is
> GLES31 on GLES31. Would it be possible to fix those cases without
> copy_sub_image??
I have to admid, I kind of ignored this, but now that you point it out:
the reason why I came up with using glCopyImageSubData is because
that's what is actually used by the tests I'm trying to fix with these
patches, and these are from the GLES31 set. So if glCopyImageSubData is
not available on the host, one can not expect these tests to pass,
right? In any case, does the hardware you're using not expose one of
the *_copy_image extensions?
Apart from that, when texture_view lands, it might also be possible to
work around a missing glCopyImageSubData, because a texture view can be
used to make a texture with one internal format pose as another one,
and then one can blit without converting. I already started to review
Dave's texture view patches, and when they land I'll try to come up
with a code path that doesn't require glCopyImageSubData.
Best,
Gert
More information about the virglrenderer-devel
mailing list