Stephane Marchesin wrote:
Brian Paul wrote:


I guess I'd like to see the clipping issues resolved before checking in the patch.


Ok, I'm not sure I understood what you expect, but here is what I did :
I used the mesa window space clipping routines and then added a dri/common/clip.h file that does the screen clipping.


Maybe someone with a Radeon card handy can test this. I have a couple comments below.


------------------------------------------------------------------------

diff -Naur ./common/clip.h 
../../../../../Mesa.read/src/mesa/drivers/dri/common/clip.h
--- ./common/clip.h     1970-01-01 01:00:00.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/common/clip.h 2004-11-11 
20:48:09.000000000 +0100
@@ -0,0 +1,35 @@
+#include "drm.h"
+

How about adding some comments to this function to explain all the parameters?



+static __inline__ void dri_read_screen_clip(int* mx1,int* mx2,int* my1,int* my2,int x,int y,int width,int height,drm_clip_rect_t *box,int nbox)
+{
+ int mw,mh,i;
+ *mx1 = box[0].x1;
+ *my1 = box[0].y1;
+ *mx2 = box[0].x2;
+ *my2 = box[0].y2;
+ mw=mx2-mx1;
+ mh=my2-my1;
+
+ /* merge the rects */
+ for (i = 1 ; i < nbox ; i++)
+ {
+ if (box[i].x1 < *mx1) *mx1 = box[i].x1;
+ if (box[i].y1 < *my1) *my1 = box[i].y1;
+ if (box[i].x2 > *mx2) *mx2 = box[i].x2;
+ if (box[i].y2 > *my2) *my2 = box[i].y2;
+ }
+ mw=*mx2-*mx1;
+ mh=*my2-*my1;
+
+ /* intersect with the area we are supposed to read */
+ if (*mx1 < x) mw -= x - *mx1, *mx1 = x;
+ if (*my1 < y) mh -= y - *my1, *my1 = y;
+ if (*mx1 + mw > x + width) mw = x + width - *mx1;
+ if (*my1 + mh > y + height) mh = y + height - *my1;
+ if (mw <= 0) return;
+ if (mh <= 0) return;
+ *mx2=*mx1+mw;
+ *my2=*my1+mh;
+}
+
+
diff -Naur ./radeon/Makefile ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/Makefile
--- ./radeon/Makefile 2004-11-11 21:44:34.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/Makefile 2004-11-03 17:07:59.000000000 +0100
@@ -22,6 +22,7 @@
radeon_context.c \
radeon_ioctl.c \
radeon_lock.c \
+ radeon_pixel.c \
radeon_screen.c \
radeon_state.c \
radeon_state_init.c \
diff -Naur ./radeon/radeon_context.c ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_context.c
--- ./radeon/radeon_context.c 2004-11-11 21:44:34.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_context.c 2004-11-08 21:00:47.000000000 +0100
@@ -343,7 +343,14 @@
ctx->Const.MaxLineWidth = 10.0;
ctx->Const.MaxLineWidthAA = 10.0;
ctx->Const.LineWidthGranularity = 0.0625;
-
+ if (screen->cpp == 4)
+ {
+ ctx->Const.ColorReadFormat = GL_BGRA;
+ ctx->Const.ColorReadType = GL_UNSIGNED_INT_8_8_8_8_REV;
+ } else {
+ ctx->Const.ColorReadFormat = GL_BGR;
+ ctx->Const.ColorReadType = GL_UNSIGNED_SHORT_5_6_5_REV;
+ }
/* Set maxlocksize (and hence vb size) small enough to avoid
* fallbacks in radeon_tcl.c. ie. guarentee that all vertices can
* fit in a single dma buffer for indexed rendering of quad strips,
diff -Naur ./radeon/radeon_context.h ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_context.h
--- ./radeon/radeon_context.h 2004-11-11 21:44:34.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_context.h 2004-11-08 21:25:50.000000000 +0100
@@ -847,6 +847,7 @@
#define DEBUG_DRI 0x200
#define DEBUG_DMA 0x400
#define DEBUG_SANITY 0x800
+#define DEBUG_PIXEL 0x2000
#endif
#endif /* __RADEON_CONTEXT_H__ */
diff -Naur ./radeon/radeon_ioctl.c ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_ioctl.c
--- ./radeon/radeon_ioctl.c 2004-11-11 21:44:34.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_ioctl.c 2004-11-08 21:01:33.000000000 +0100
@@ -59,7 +59,7 @@
static void radeonWaitForIdle( radeonContextPtr rmesa );
-static int radeonFlushCmdBufLocked( radeonContextPtr rmesa, +int radeonFlushCmdBufLocked( radeonContextPtr rmesa, const char * caller );
static void radeonSaveHwState( radeonContextPtr rmesa )
@@ -518,7 +518,7 @@
}
-static int radeonFlushCmdBufLocked( radeonContextPtr rmesa, +int radeonFlushCmdBufLocked( radeonContextPtr rmesa, const char * caller )
{
int ret, i;
@@ -752,6 +752,8 @@
rmesa->dma.current.ptr += bytes; /* bug - if alignment > 7 */
rmesa->dma.current.start = rmesa->dma.current.ptr = (rmesa->dma.current.ptr + 0x7) & ~0x7; + + assert( rmesa->dma.current.ptr <= rmesa->dma.current.end );
}
void radeonAllocDmaRegionVerts( radeonContextPtr rmesa, @@ -1233,6 +1235,30 @@
radeonWaitForIdle( rmesa );
}
+GLboolean radeonIsGartMemory( radeonContextPtr rmesa, const GLvoid *pointer,
+ GLint size )
+{
+ ptrdiff_t offset = (char *)pointer - (char *)rmesa->radeonScreen->gartTextures.map;
+ int valid = (size >= 0 &&
+ offset >= 0 &&
+ offset + size < rmesa->radeonScreen->gartTextures.size);
+ + if (RADEON_DEBUG & DEBUG_IOCTL)
+ fprintf(stderr, "radeonIsGartMemory( %p ) : %d\n", pointer, valid );
+ + return valid;
+}
+
+GLuint radeonGartOffsetFromVirtual( radeonContextPtr rmesa, const GLvoid *pointer )
+{
+ ptrdiff_t offset = (char *)pointer - (char *)rmesa->radeonScreen->gartTextures.map;
+
+ if (offset < 0 || offset > rmesa->radeonScreen->gartTextures.size)
+ return ~0;
+ else
+ return rmesa->radeonScreen->gart_texture_offset + offset;
+}
+
void radeonInitIoctlFuncs( GLcontext *ctx )
{
diff -Naur ./radeon/radeon_ioctl.h ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_ioctl.h
--- ./radeon/radeon_ioctl.h 2004-11-11 21:44:34.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_ioctl.h 2004-11-03 17:04:48.000000000 +0100
@@ -77,6 +77,8 @@
extern void radeonEmitWait( radeonContextPtr rmesa, GLuint flags );
+extern int radeonFlushCmdBufLocked( radeonContextPtr rmesa, + const char * caller );
extern void radeonFlushCmdBuf( radeonContextPtr rmesa, const char * );
extern void radeonRefillCurrentDmaRegion( radeonContextPtr rmesa );
@@ -99,6 +101,12 @@
extern void radeonPageFlip( const __DRIdrawablePrivate *drawable );
extern void radeonFlush( GLcontext *ctx );
extern void radeonFinish( GLcontext *ctx );
+extern GLboolean radeonIsGartMemory( radeonContextPtr rmesa, + const GLvoid *pointer,
+ GLint size );
+extern GLuint radeonGartOffsetFromVirtual( radeonContextPtr rmesa, + const GLvoid *pointer );
+ extern void radeonWaitForIdleLocked( radeonContextPtr rmesa );
extern void radeonWaitForVBlank( radeonContextPtr rmesa );
extern void radeonInitIoctlFuncs( GLcontext *ctx );
diff -Naur ./radeon/radeon_pixel.c ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_pixel.c
--- ./radeon/radeon_pixel.c 1970-01-01 01:00:00.000000000 +0100
+++ ../../../../../Mesa.read/src/mesa/drivers/dri/radeon/radeon_pixel.c 2004-11-11 21:42:57.000000000 +0100
@@ -0,0 +1,246 @@
+#include "glheader.h"
+#include "enums.h"
+#include "mtypes.h"
+#include "macros.h"
+#include "clip.h"
+#include "image.h"
+#include "swrast/swrast.h"
+
+#include "radeon_context.h"
+#include "radeon_ioctl.h"
+#include "radeon_pixel.h"
+#include "radeon_swtcl.h"
+
+
+static GLboolean
+check_color( const GLcontext *ctx, GLenum type, GLenum format,
+ const struct gl_pixelstore_attrib *packing,
+ const void *pixels, GLint sz )
+{
+ radeonContextPtr rmesa = RADEON_CONTEXT(ctx);
+ GLuint cpp = rmesa->radeonScreen->cpp;
+
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s\n", __FUNCTION__);
+
+ if ( ctx->_ImageTransferState ||
+ packing->SwapBytes ||
+ packing->LsbFirst) {
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s: failed 1\n", __FUNCTION__);
+ return GL_FALSE;
+ }
+
+ if ( type == GL_UNSIGNED_SHORT_5_6_5_REV && + cpp == 2 && + format == GL_BGR ) {
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s: passed 2\n", __FUNCTION__);
+ return GL_TRUE;
+ }
+
+ if ( type == GL_UNSIGNED_INT_8_8_8_8_REV && + cpp == 4 && + format == GL_BGRA ) {
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s: passed 2\n", __FUNCTION__);
+ return GL_TRUE;
+ }
+ + if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s: failed\n", __FUNCTION__);
+
+ return GL_FALSE;
+}
+
+
+static void do_read_pix( GLcontext *ctx,
+ GLint x, GLint y, GLsizei width, GLsizei height,
+ GLint pitch,
+ const void *pixels,
+ int blit_format)
+{
+ radeonContextPtr rmesa = RADEON_CONTEXT(ctx);
+ __DRIdrawablePrivate *dPriv = rmesa->dri.drawable;
+ int nbox = dPriv->numClipRects;
+ int src_offset = rmesa->state.color.drawOffset
+ + rmesa->radeonScreen->fbLocation;
+ int dst_offset;
+ drm_clip_rect_t *box = dPriv->pClipRects;
+ int src_pitch = rmesa->state.color.drawPitch * rmesa->radeonScreen->cpp;
+ int dst_pitch = pitch;
+ GLint mx1,mx2,my1,my2;
+ + if (!nbox)
+ return;
+
+ y = dPriv->h - y - height;
+ x += dPriv->x;
+ y += dPriv->y;
+
+
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "readpixel blit src_pitch %d dst_pitch %d\n",
+ src_pitch, dst_pitch);
+
+ dri_read_screen_clip(&mx1,&mx2,&my1,&my2,x,y,width,height,box,nbox),
+ + dst_offset = radeonGartOffsetFromVirtual( rmesa, pixels );
+ radeonEmitWait( rmesa, RADEON_WAIT_3D ); +
+ radeonEmitBlit( rmesa,
+ blit_format,
+ src_pitch, src_offset,
+ dst_pitch, dst_offset,
+ mx1, my1,
+ mx1 - x, my1 - y,
+ mx2 - mx1, my2 - my1 );
+
+ radeonFlushCmdBufLocked( rmesa, __FUNCTION__ );
+}
+
+static GLboolean
+radeonTryReadPixels( GLcontext *ctx,
+ GLint x, GLint y, GLsizei width, GLsizei height,
+ GLenum format, GLenum type,
+ const struct gl_pixelstore_attrib *pack,
+ GLvoid *pixels )
+{
+ radeonContextPtr rmesa = RADEON_CONTEXT(ctx);
+ GLuint cpp = rmesa->radeonScreen->cpp;
+ GLint pitch = (pack->RowLength ? pack->RowLength : width) * cpp;
+ GLint size;
+ GLint blit_format;
+ + pitch= (pitch+63)&~63;
+
+ size=pitch * height;
+
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s\n", __FUNCTION__);
+
+ /* this is the threshold above which a blit is faster than video memory acccess by the CPU + * tuning for this is approximate */
+ if (size<64)
+ return GL_FALSE;
+
+ if (!check_color(ctx, type, format, pack, pixels, size))
+ return GL_FALSE;
+
+ switch ( rmesa->radeonScreen->cpp ) {
+ case 2:
+ blit_format = RADEON_GMC_DST_16BPP;
+ break;
+ case 4:
+ blit_format = RADEON_GMC_DST_32BPP;
+ break;
+ default:
+ return GL_FALSE;
+ }
+
+ + /* Although the blits go on the command buffer, need to do this and
+ * fire with lock held to guarentee cliprects and drawOffset are
+ * correct.
+ *
+ * This is an unusual situation however, as the code which flushes
+ * a full command buffer expects to be called unlocked. As a
+ * workaround, immediately flush the buffer on aquiring the lock.
+ */
+ LOCK_HARDWARE( rmesa );
+
+ if (rmesa->store.cmd_used)
+ radeonFlushCmdBufLocked( rmesa, __FUNCTION__ );
+ + {
+ /* Pixels is in regular memory -- get dma buffers and perform
+ * upload through them.
+ */
+ + int ret;
+ int region_offset;
+ drm_radeon_mem_alloc_t alloc;
+ drm_radeon_mem_free_t memfree;
+ char* region;
+ int yl;
+
+ alloc.region = RADEON_MEM_REGION_GART;
+ alloc.alignment = 1024;
+ alloc.size = size;
+ alloc.region_offset = &region_offset;
+
+ ret = drmCommandWriteRead( rmesa->radeonScreen->driScreen->fd,
+ DRM_RADEON_ALLOC,
+ &alloc, sizeof(alloc));
+ if (ret) {
+ UNLOCK_HARDWARE( rmesa );
+ return GL_FALSE;
+ }
+ region =(void *)((char *)rmesa->radeonScreen->gartTextures.map + region_offset);
+ + do_read_pix( ctx, x, y, width, height, pitch, region, blit_format);
+ UNLOCK_HARDWARE( rmesa );
+ radeonFinish( ctx ); /* required for GL */
+
+ /* the thing is upside down on the card, swap it */
+ for (yl=0;yl<height;yl++)
+ {
+ memcpy(((char*)pixels)+yl*(pack->RowLength ? pack->RowLength : width)*cpp, + (void *)((char *)rmesa->radeonScreen->gartTextures.map + region_offset + (height-1-yl)*pitch), + (pack->RowLength ? pack->RowLength : width)*cpp);
+ } + + memfree.region = RADEON_MEM_REGION_GART;
+ memfree.region_offset = region_offset;
+
+ ret = drmCommandWrite( rmesa->radeonScreen->driScreen->fd,
+ DRM_RADEON_FREE,
+ &memfree, sizeof(memfree));
+ if (ret)
+ fprintf(stderr, "%s: DRM_RADEON_FREE ret %d\n", __FUNCTION__, ret);
+ return GL_TRUE;
+ }
+
+}
+
+static void
+radeonReadPixels( GLcontext *ctx,
+ GLint x, GLint y, GLsizei width, GLsizei height,
+ GLenum format, GLenum type,
+ const struct gl_pixelstore_attrib *pack,
+ GLvoid *pixels )
+{
+ if (RADEON_DEBUG & DEBUG_PIXEL)
+ fprintf(stderr, "%s\n", __FUNCTION__);
+
+ if (!_mesa_clip_readpixels(ctx, &x, &y, &width, &height,
+ &pack->SkipPixels,
+ &pack->SkipRows)) {

Don't you get compiler warnings here? The 'pack' pointer is const-qualified but last two parameters of _mesa_clip_readpixels() are not const-qualified. I.e. you should _not_ be modifying the fields of the 'pack' object. You should make a copy of the struct first.



-Brian


------------------------------------------------------- This SF.Net email is sponsored by: InterSystems CACHE FREE OODBMS DOWNLOAD - A multidimensional database that combines robust object and relational technologies, making it a perfect match for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8 -- _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to