<html>
    <head>
      <base href="https://bugs.freedesktop.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - _mesa_meta_GenerateMipmap sub-optimal code and possible bug"
   href="https://bugs.freedesktop.org/show_bug.cgi?id=103672">103672</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>_mesa_meta_GenerateMipmap sub-optimal code and possible bug
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>Mesa
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>unspecified
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>normal
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>medium
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>Mesa core
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>mesa-dev@lists.freedesktop.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>weilercdale@gmail.com
          </td>
        </tr>

        <tr>
          <th>QA Contact</th>
          <td>mesa-dev@lists.freedesktop.org
          </td>
        </tr></table>
      <p>
        <div>
        <pre>Was just reviewing this code since I hit a pathological edge case where it was
severly impacting the performance of generating mip levels and was surprised to
see the approach taken.

Couple of alarming issues:

1. It uploads vertex data for every mip level to draw a quad, when this data
only needs to be uploaded once, lifting that out of the for loop is a trivial
one line change but will need further changes because of the
_mesa_meta_setup_texture_coords stuff, probably best to create one large vertex
buffer for all of this upfront instead.

2. It allocates storage for every mip level one iteration at a time when it
couple upfront allocate all the storage needed to represent all mip levels in
one shot.

3. When setting minification to the source level, I'm sure this is a copy-paste
bug (since it's called above to set max level but then called again), but
appears to use GL_TEXTURE_MAX_LEVEL instead of GL_TEXTURE_MIN_LEVEL.

4. The comment says it does not support 3D textures yet but it appears the code
does handle 3D textures.

5. The approach itself seems like it could better be done via glCopyTexImage*
family of functions which already take from the read buffer, thus avoiding ugly
vbo/vao state changes (as well as viewport changes) and issuing a draw call for
each mip level.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the QA Contact for the bug.</li>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>