<div dir="ltr">On 5 February 2013 11:34, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On 4 February 2013 21:48, Chris Forbes <span dir="ltr"><<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This series adds the core mesa bits and i965 driver support for<br>
ARB_texture_multisample.<br>
<br>
Big changes from V1:<br>
   - Generally cleaner<br>
   - GLSL texelFetch() with a multisample sampler is converted to<br>
      a new ir_txf_ms opcode in the GLSL IR, which makes the driver side<br>
      a lot tidier.<br>
   - Most of the review comments have been addressed.<br>
   - Intel driver support includes both Gen6 and Gen7 now.<br>
   - Lots of bugs fixed.<br>
<br>
Remaining issues:<br>
   - Stencil support is still a mess. This is disabled in the intel driver<br>
     for now by exposing only GL_MAX_DEPTH_TEXTURE_SAMPLES=1. To do this<br>
     properly requires some more small changes in core mesa to cope with<br>
     stencil-only texture formats a bit better, and Intel-specific support<br>
     to cope with the strange W-tiled stencil buffer format.<br>
<br>
   - The IvyBridge support forces uncompressed multisample layout, which<br>
     is wasteful of memory bandwidth. Allowing CMS here requires emitting<br>
     specialized code based on the multisample layout, since fetching from<br>
     an undefined MCS surface isn't safe.<br>
<br>
   - It's unclear whether deallocating multisample textures via zero<br>
     dimensions is legal, or whether the texture object must be deleted.<br>
<br>
I'm going to continue working on these issues, but would appreciate another<br>
round of review comments in the mean time.<br>
<br>
-- Chris<br></blockquote><div><br></div></div></div><div>I'm about halfway through reviewing the series, and out of time for patch review today.  I'll pick it up again tomorrow.  So far it looks really good!  I think we are nearing the home stretch on this series.  Thanks again for your continued efforts, Chris.  I'm going to be really glad to see this feature land.<br>

</div></div></div></div>
</blockquote></div><br></div><div class="gmail_extra">Ok, finished my review.  I sent comments on patches 10, 15, 18, 20, and 21.  Also, I second Eric's comment that patches 1-3 need to be squashed--we try to keep Mesa buildable (and passing "make check") after every commit, for ease in future bisecting.<br>
<br>The remaining patches are:<br><br></div><div class="gmail_extra">Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br><br></div><div class="gmail_extra">Note: considering the complexity of this series, I tihnk it would be good to try to land the Piglit tests at the same time as the feature.<br>
</div></div>