[Mesa-dev] [PATCH] docs: Document spec quoting practices

[Ian Romanick]

On 12/19/2012 09:56 PM, Paul Berry wrote:
> On 19 December 2012 15:46, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
>     From: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>
>     Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>     Cc: Paul Berry <stereotype441 at gmail.com
>     <mailto:stereotype441 at gmail.com>>
>     Cc: Brian Paul <brianp at vmware.com <mailto:brianp at vmware.com>>
>     ---
>       docs/devinfo.html | 42 ++++++++++++++++++++++++++++++++++++++++++
>       1 file changed, 42 insertions(+)
>
>     diff --git a/docs/devinfo.html b/docs/devinfo.html
>     index 8f4aeef..eb4c897 100644
>     --- a/docs/devinfo.html
>     +++ b/docs/devinfo.html
>     @@ -155,6 +155,48 @@ of <tt>bool</tt>, <tt>true</tt>, and
>       src/mesa/state_tracker/st_glsl_to_tgsi.cpp can serve as examples.
>       </p>
>
>     +<p>
>     +It is often useful to quote sections from relevant specifications near
>     +code that implements part of the spec.  In order to make it easier to
>     +find such quotations in the code (via grep and friends) and to find the
>     +quoted text in the specifications, please use one of the following
>     +formats.
>     +</p>
>     +
>     +<p>
>     +Text quoted from the core OpenGL or OpenGL ES specifications:
>     +</p>
>     +
>     +<pre>
>     +   /* Page AA (page BB of the PDF) in section C.D.E of the OpenGL F.G
>     +    * specification says:
>     +    *
>     +    *     "Some quoted text from the specificiation"
>     +    */
>     +</pre>
>
>
> The OpenGL specs since version 3.2 come in four flavours:
> - glspecN.core.YYYYMMDD.pdf
> - glspecN.core.YYYYMMDD.withchanges.pdf
> - glspecN.compatibility.YYYYMMDD.pdf
> - glspecN.compatibility.YYYYMMDD.withchanges.pdf
>
> Since these four documents don't in general have matching page numbers,
> I think we should make a recommendation as to which one to quote from.
> My preference: glspecN.compatibility.YYYYMMDD.withchanges.pdf, since it
> contains information about the deltas both from core to compatibility
> and from one version to the next, so it's the most likely to tip us off
> to subtle API or version dependencies that we need to be careful about.

Except that we don't implement compatibility profiles. :)  I agree that 
in cases where a profile may be involved, the reference should call it 
out.  You're thinking it should look like:

     /* Page AA (page BB of the PDF) in section C.D.E of
      * glspecN.compatibility.YYYYMMDD.withchanges.pdf says:
      *
      *     "Some quoted text from the specificiation"
      */

That's very specific, but it's also kind of ugly to look at.  Hmm...

>     +
>     +<p>
>     +Text quoted from the GLSL or GLSL ES ES specifications:
>     +</p>
>     +
>     +<pre>
>     +   /* Page AA (page BB of the PDF) in section C.D.E of the GLSL F.G
>     +    * specification says:
>     +    *
>     +    *     "Some quoted text from the specificiation"
>     +    */
>     +</pre>
>     +
>     +<p>
>     +Text quoted from an extension specifications:
>     +</p>
>     +
>     +<pre>
>     +   /* Section C.D.E of the GL_EXT_foo_bar specification says:
>     +    *
>     +    *     "Some quoted text from the specificiation"
>     +    */
>     +</pre>
>
>
> My impression is that most extension specs aren't divided into numbered
> sections like this--instead the sections tend to be labeled things like
> "Overview", "Issues", or "Additions to Chapter N...".  Since extension

This may be a case of under-specifying what I meant.  In this context, 
by "Section C.D.E" I meant the section of the core spec that the 
extension text modifies.  I may have to think about the right way to say 
this one a bit more...

Right now there is a comment in invalidate_framebuffer_storage 
(fboject.c) that says:

    /* The GL_ARB_invalidate_subdata spec says:
     *
     *     "If an attachment is specified that does not exist in the
     *     framebuffer bound to <target>, it is ignored."
     *
     * It also says:
     *
     *     "If <attachments> contains COLOR_ATTACHMENTm and m is greater 
than
     *     or equal to the value of MAX_COLOR_ATTACHMENTS, then the error
     *     INVALID_OPERATION is generated."
     *
     * No mention is made of GL_AUXi being out of range.  Therefore, we 
allow
     * any enum that can be allowed by the API (OpenGL ES 3.0 has a 
different
     * set of retrictions).
     */

I'm proposing that this should instead say:

    /* Section 4.5 of the GL_ARB_invalidate_subdata spec says:
     *
     *     "If an attachment is specified that does not exist in the
     *     framebuffer bound to <target>, it is ignored."
     *
     * It also says:
     *
     *     "If <attachments> contains COLOR_ATTACHMENTm and m is greater 
than
     *     or equal to the value of MAX_COLOR_ATTACHMENTS, then the error
     *     INVALID_OPERATION is generated."
     *
     * No mention is made of GL_AUXi being out of range.  Therefore, we 
allow
     * any enum that can be allowed by the API (OpenGL ES 3.0 has a 
different
     * set of retrictions).
     */

Though, after looking at this case and a few other cases, I'm not sure 
there's much value in mentioning the section.  Many of the extension 
specs that I checked only modify a single section in a substantive manner.

> specifications are text files, perhaps it would be better to quote by
> line number?

Quoting by line number seems like it would be annoying.  The extension 
specs are (usually) small enough that searching in the document for the 
text should be good enough to fine-tune the location.

>       <h2>Marking a commit as a candidate for a stable branch</h2>
>
>     --
>     1.7.11.7