On Wed 08 Nov 2017, Jason Ekstrand wrote: > The old code made a new miptree that referenced the same BO as the > renderbuffer and just trusted in the memory aliasing to work. There are > only two ways in which the new miptree is liable to differ from the one > in the renderbuffer and neither of them matter:
Ouch. I didn't realize intelSetTexBuffer2 created a new miptree, but I'm not surprised. > > 1) It may have a different target. The only targets that we can ever > see in intelSetTexBuffer2 are GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE > and the difference between the two doesn't matter as far as the > miptree is concerned; genX(update_sampler_state) only looks at the > gl_texture_object and not the miptree when determining whether or > not to use normalized coordinates. > > 2) It may have a very slightly different format. Again, this doesn't > matter because we've supported texture views for quite some time so > we always look at the gl_texture_object format instead of the > miptree format for hardware setup anyway. > > On the other hand, because we were recreating the miptree, we were using > intel_miptree_create_for_bo which doesn't understand modifiers. We > really want this function to work without doing a resolve so long as you > have modifiers so we need to fix that. > > Cc: "17.3" <[email protected]> > --- > src/mesa/drivers/dri/i965/intel_tex_image.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c > b/src/mesa/drivers/dri/i965/intel_tex_image.c > index 37c8e24..c52992a 100644 > --- a/src/mesa/drivers/dri/i965/intel_tex_image.c > +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c > @@ -405,6 +405,7 @@ static void > intel_set_texture_image_mt(struct brw_context *brw, > struct gl_texture_image *image, > GLenum internal_format, > + mesa_format format, > struct intel_mipmap_tree *mt) > > { > @@ -415,7 +416,7 @@ intel_set_texture_image_mt(struct brw_context *brw, > _mesa_init_teximage_fields(&brw->ctx, image, > mt->surf.logical_level0_px.width, > mt->surf.logical_level0_px.height, 1, > - 0, internal_format, mt->format); > + 0, internal_format, format); > > brw->ctx.Driver.FreeTextureImageBuffer(&brw->ctx, image); > > @@ -442,7 +443,6 @@ intelSetTexBuffer2(__DRIcontext *pDRICtx, GLint target, > struct gl_texture_object *texObj; > struct gl_texture_image *texImage; > mesa_format texFormat = MESA_FORMAT_NONE; > - struct intel_mipmap_tree *mt; > GLenum internal_format = 0; > > texObj = _mesa_get_current_tex_object(ctx, target); > @@ -464,31 +464,24 @@ intelSetTexBuffer2(__DRIcontext *pDRICtx, GLint target, > if (rb->mt->cpp == 4) { > if (texture_format == __DRI_TEXTURE_FORMAT_RGB) { > internal_format = GL_RGB; > - texFormat = MESA_FORMAT_B8G8R8X8_UNORM; > + texFormat = MESA_FORMAT_B8G8R8A8_UNORM; Why replace rgbx with rgba? I suspect the replace is due to the same reasons explained in intel_miptree_create_for_dri_image(). Whatever the reasons are, they're subtle and deserve a comment. For reference, here is the rgbx->rgba replacement in intel_miptree_create_for_dri_image: if (!brw->ctx.TextureFormatSupported[format]) { /* The texture storage paths in core Mesa detect if the driver does not * support the user-requested format, and then searches for a * fallback format. The DRIimage code bypasses core Mesa, though. So we * do the fallbacks here for important formats. * * We must support DRM_FOURCC_XBGR8888 textures because the Android * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces, which * the Chrome OS compositor consumes as dma_buf EGLImages. */ format = _mesa_format_fallback_rgbx_to_rgba(format); } > } > else { > internal_format = GL_RGBA; > texFormat = MESA_FORMAT_B8G8R8A8_UNORM; > } > } else if (rb->mt->cpp == 2) { > + /* This is 565 */ > internal_format = GL_RGB; > texFormat = MESA_FORMAT_B5G6R5_UNORM; > } > > intel_miptree_make_shareable(brw, rb->mt); > - mt = intel_miptree_create_for_bo(brw, rb->mt->bo, texFormat, 0, > - rb->Base.Base.Width, > - rb->Base.Base.Height, > - 1, rb->mt->surf.row_pitch, > - MIPTREE_CREATE_DEFAULT); > - if (mt == NULL) > - return; > - mt->target = target; > > _mesa_lock_texture(&brw->ctx, texObj); > texImage = _mesa_get_tex_image(ctx, texObj, target, 0); > - intel_set_texture_image_mt(brw, texImage, internal_format, mt); > - intel_miptree_release(&mt); > + intel_set_texture_image_mt(brw, texImage, internal_format, > + texFormat, rb->mt); > _mesa_unlock_texture(&brw->ctx, texObj); > } > > @@ -581,7 +574,7 @@ intel_image_target_texture_2d(struct gl_context *ctx, > GLenum target, > const GLenum internal_format = > image->internal_format != 0 ? > image->internal_format : _mesa_get_format_base_format(mt->format); > - intel_set_texture_image_mt(brw, texImage, internal_format, mt); > + intel_set_texture_image_mt(brw, texImage, internal_format, mt->format, > mt); This hunk looks correct. mt->format is the only suitable format available in this function. > intel_miptree_release(&mt); > } > > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
