Nicolai Hähnle wrote: > Am Thursday 15 October 2009 22:50:21 schrieb Brian Paul: >> Nicolai Hähnle wrote: >>> I sent the last mail a little too quickly: This seems to affect only >>> certain programs, in particular I have seen it with Sauerbraten; Compiz >>> on the other hand seems to work. >> I've committed another change that should fix that. > > Thank you. Indeed, the games and applications I've tested all work fine. > > Unfortunately, there's a problem with swrast access (in particular > ReadPixels, > which causes readPixSanity to fail). Can you clarify what each of the fields > ::_BaseFormat, ::InternalFormat and ::Format are supposed to mean, and how > they interact with GetRow, PutValues, etc.?
(sorry for the slow reply) InternalFormat is the user value passed to glRenderbufferStorage. _BaseFormat is the InternalFormat boiled down to one of GL_RGBA, GL_DEPTH_COMPONENT, GL_STENCIL_INDEX, etc. Format is the actual hardware format for the buffer. The GetRow(), etc. functions generally return either RGBA or stencil or depth values according to _BaseFormat. The DataType field indicates the datatype for the values (GL_UNSIGNED_BYTE, GL_FLOAT, etc) passed through Get/PutRow(). > My observations: > 1. Your change to radeon_span.c indicates that you want GetRow to depend on > ::Format, but ::Format does not distinguish between stencil and depth buffer > access when Format == MESA_FORMAT_S8_Z24. Hmmm, it should be the equivalent of what we had before with the obsolete _ActualFormat field. When core Mesa needs to read Z or stencil values out of a combined Z+Stencil buffer we use renderbuffer "adaptors" which wrap the underlying buffer and make it look like a Z-only or Stencil-only buffer. See main/depthstencil.c. > 2. The assertion in s_readpix.c:122 gets triggered because rb->InternalFormat > == GL_DEPTH24_STENCIL8 happens. I think that assertion will need to be removed. At that point what does *rb look like (gdb: print *rb). > 3. The stencil read code expects GetRow to return only stencil values even > when ::Format is one of the mixed depth/stencil formats. Again, the renderbuffer depth/stencil adaptors above should take care of this. fb->_StencilBuffer should point to a stencil wrapper around the combined depth/stencil buffer. These wrappers are installed during framebuffer state validation in _mesa_update_depth_buffer() and _mesa_update_stencil_buffer(). I just noticed that tests/zreaddraw.c fails with the i965 driver. It may be something similar to the radeon issue. > So what is really the way to go forward? It seems radeon_span.c should > probably use ::_BaseFormat to decide whether Mesa wants depth or stencil > access functions, use ::InternalFormat to decide the data type that Mesa > wants > to use (e.g. GL_DEPTH24_STENCIL8 and GL_DEPTH_COMPONENT24 both mean 24-bit Z > values if _BaseFormat == GL_DEPTH_COMPONENT, whereas GL_DEPTH_COMPONENT16 > means 16-bit Z values) and then pick the particular family of access > functions > based on the ::Format. Is that correct? > > Are there invariants of the form: "If Format == MESA_FORMAT_S8_Z24, then > InternalFormat is always GL_DEPTH24_STENCIL8"? That example isn't true because the InternalFormat comes from the user and could be one of several values. One of the next steps I want to make after the texformat-rework is removing the Get/PutRow() functions and move toward a Map/Unmap model where drivers won't have to worry about span functions at all. The span functions go back to the very first version of Mesa circa 1993. -Brian ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Mesa3d-dev mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
