Re: [Mesa-dev] [PATCH 4/7] dri2: Trust our own driver name lookup over the server's.
This patch could cause the i965 driver to not load if Mesa was built on a system without libudev devel present. For example on Fedora one should install systemd-devel before configuring and building Mesa drivers subsequent to this change. On Sun, Jan 26, 2014 at 4:02 PM, Keith Packard kei...@keithp.com wrote: Eric Anholt e...@anholt.net writes: This allows Mesa to choose to rename driver .sos (or split drivers), without needing a flag day with the corresponding 2D driver. Reviewed-by: Keith Packard kei...@keithp.com -- keith.pack...@intel.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Add warning to _REV pack/unpack functions with incorrect behavior
Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/main/format_pack.c | 7 ++- src/mesa/main/format_unpack.c | 12 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c index 39d767a..4c7ed58 100644 --- a/src/mesa/main/format_pack.c +++ b/src/mesa/main/format_pack.c @@ -492,6 +492,8 @@ pack_row_float_RGB565(GLuint n, const GLfloat src[][4], void *dst) /* * MESA_FORMAT_R5G6B5_UNORM + * Warning: these functions do not match the current Mesa definition + * of MESA_FORMAT_R5G6B5_UNORM. */ static void @@ -621,7 +623,10 @@ pack_float_ARGB1555(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_A1R5G5B5_UNORM */ +/* MESA_FORMAT_A1R5G5B5_UNORM + * Warning: these functions do not match the current Mesa definition + * of MESA_FORMAT_A1R5G5B5_UNORM. + */ static void pack_ubyte_ARGB1555_REV(const GLubyte src[4], void *dst) diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c index 52aa7d2..dc9eba7 100644 --- a/src/mesa/main/format_unpack.c +++ b/src/mesa/main/format_unpack.c @@ -234,6 +234,9 @@ unpack_RGB565(const void *src, GLfloat dst[][4], GLuint n) static void unpack_RGB565_REV(const void *src, GLfloat dst[][4], GLuint n) { + /* Warning: this function does not match the current Mesa definition +* of MESA_FORMAT_R5G6B5_UNORM. +*/ const GLushort *s = ((const GLushort *) src); GLuint i; for (i = 0; i n; i++) { @@ -300,6 +303,9 @@ unpack_ARGB1555(const void *src, GLfloat dst[][4], GLuint n) static void unpack_ARGB1555_REV(const void *src, GLfloat dst[][4], GLuint n) { + /* Warning: this function does not match the current Mesa definition +* of MESA_FORMAT_A1R5G5B5_UNORM. +*/ const GLushort *s = ((const GLushort *) src); GLuint i; for (i = 0; i n; i++) { @@ -2699,6 +2705,9 @@ unpack_ubyte_RGB565(const void *src, GLubyte dst[][4], GLuint n) static void unpack_ubyte_RGB565_REV(const void *src, GLubyte dst[][4], GLuint n) { + /* Warning: this function does not match the current Mesa definition +* of MESA_FORMAT_R5G6B5_UNORM. +*/ const GLushort *s = ((const GLushort *) src); GLuint i; for (i = 0; i n; i++) { @@ -2765,6 +2774,9 @@ unpack_ubyte_ARGB1555(const void *src, GLubyte dst[][4], GLuint n) static void unpack_ubyte_ARGB1555_REV(const void *src, GLubyte dst[][4], GLuint n) { + /* Warning: this function does not match the current Mesa definition +* of MESA_FORMAT_A1R5G5B5_UNORM. +*/ const GLushort *s = ((const GLushort *) src); GLuint i; for (i = 0; i n; i++) { -- 1.8.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification
On Fri, Jan 24, 2014 at 2:50 AM, Marek Olšák mar...@gmail.com wrote: On Thu, Jan 23, 2014 at 8:36 PM, Kenneth Graunke kenn...@whitecape.org wrote: On 01/23/2014 11:19 AM, Mark Mueller wrote: That works for sRGB, but what about other color spaces that may be added in the future? I'm fine with leaving that to another day and with using the _SRGB decoration, as you say. Oy. Let's not worry about things that hypothetically could be someday. We need to do what makes sense today. Yeah, let's commit this. We can discuss the other things later, including if SRGB should be a type or a modifier. The series is already pretty good. Marek I'm not sure if further acknowledgement is needed to push the series, but, either way, I'm not authorized to push these. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [V5 PATCH 0/7] Naming MESA_FORMATs to a specification
This series introduces a specification for 3 types of MESA_FORMAT names - Type A (array formats), Type C (compressed formats), and Type P (packed formats), and then performs a series of substitutions grouped by type. Builds of all default gallium and DRI drivers were verified and no regressions were observed w/piglit tests on the i965 driver. The format_unpack functions were used to verify component orders, except with some _REV formats, which appeared to be incorrect, but not used in normal testing. V5 is can be applied to the current master and uses the _SRGB storage type Mark Mueller (7): 1 s/\bgl_format\b/mesa_format/g. Use better name for Mesa Formats enum 2 Change all 4 color component unsigned byte formats to meet spec for P Type formats: s/MESA_FORMAT_RGBA\b/MESA_FORMAT_A8B8G8R8_UNORM/g s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_R8G8B8A8_UNORM/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_B8G8R8A8_UNORM/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A8R8G8B8_UNORM/g s/MESA_FORMAT_RGBX\b/MESA_FORMAT_X8B8G8R8_UNORM/g s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_R8G8B8X8_UNORM/g s/MESA_FORMAT_XRGB\b/MESA_FORMAT_B8G8R8X8_UNORM/g s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_X8R8G8B8_UNORM/g 3 Update comments. Conversion of the following Type A formats: s/MESA_FORMAT_RGB888\b/MESA_FORMAT_BGR_UNORM8/g s/MESA_FORMAT_BGR888\b/MESA_FORMAT_RGB_UNORM8/g s/MESA_FORMAT_A8\b/MESA_FORMAT_A_UNORM8/g s/MESA_FORMAT_A16\b/MESA_FORMAT_A_UNORM16/g s/MESA_FORMAT_L8\b/MESA_FORMAT_L_UNORM8/g s/MESA_FORMAT_L16\b/MESA_FORMAT_L_UNORM16/g s/MESA_FORMAT_I8\b/MESA_FORMAT_I_UNORM8/g s/MESA_FORMAT_I16\b/MESA_FORMAT_I_UNORM16/g s/MESA_FORMAT_R8\b/MESA_FORMAT_R_UNORM8/g s/MESA_FORMAT_R16\b/MESA_FORMAT_R_UNORM16/g s/MESA_FORMAT_Z16\b/MESA_FORMAT_Z_UNORM16/g s/MESA_FORMAT_Z32\b/MESA_FORMAT_Z_UNORM32/g s/MESA_FORMAT_S8\b/MESA_FORMAT_S_UINT8/g s/MESA_FORMAT_SRGB8\b/MESA_FORMAT_BGR_SRGB8/g s/MESA_FORMAT_RGBA_16\b/MESA_FORMAT_RGBA_UNORM16/g s/MESA_FORMAT_SL8\b/MESA_FORMAT_L_SRGB8/g s/MESA_FORMAT_Z32_FLOAT\b/MESA_FORMAT_Z_FLOAT32/g s/MESA_FORMAT_XBGR16161616_UNORM\b/MESA_FORMAT_RGBX_UNORM16/g s/MESA_FORMAT_XBGR16161616_SNORM\b/MESA_FORMAT_RGBX_SNORM16/g s/MESA_FORMAT_XBGR16161616_FLOAT\b/MESA_FORMAT_RGBX_FLOAT16/g s/MESA_FORMAT_XBGR16161616_UINT\b/MESA_FORMAT_RGBX_UINT16/g s/MESA_FORMAT_XBGR16161616_SINT\b/MESA_FORMAT_RGBX_SINT16/g s/MESA_FORMAT_XBGR32323232_FLOAT\b/MESA_FORMAT_RGBX_FLOAT32/g s/MESA_FORMAT_XBGR32323232_UINT\b/MESA_FORMAT_RGBX_UINT32/g s/MESA_FORMAT_XBGR32323232_SINT\b/MESA_FORMAT_RGBX_SINT32/g s/MESA_FORMAT_XBGR_UINT\b/MESA_FORMAT_RGBX_UINT8/g s/MESA_FORMAT_XBGR_SINT\b/MESA_FORMAT_RGBX_SINT8/g 4 Conversion of Type P formats as follows (w/related comment fixes): s/MESA_FORMAT_RGB565\b/MESA_FORMAT_B5G6R5_UNORM/g s/MESA_FORMAT_RGB565_REV\b/MESA_FORMAT_R5G6B5_UNORM/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_B4G4R4A4_UNORM/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A4R4G4B4_UNORM/g s/MESA_FORMAT_RGBA5551\b/MESA_FORMAT_A1B5G5R5_UNORM/g s/MESA_FORMAT_XBGR_SNORM\b/MESA_FORMAT_R8G8B8X8_SNORM/g s/MESA_FORMAT_XBGR_SRGB\b/MESA_FORMAT_R8G8B8X8_SRGB/g s/MESA_FORMAT_ARGB1555\b/MESA_FORMAT_B5G5R5A1_UNORM/g s/MESA_FORMAT_ARGB1555_REV\b/MESA_FORMAT_A1R5G5B5_UNORM/g s/MESA_FORMAT_AL44\b/MESA_FORMAT_L4A4_UNORM/g s/MESA_FORMAT_RGB332\b/MESA_FORMAT_B2G3R3_UNORM/g s/MESA_FORMAT_ARGB2101010\b/MESA_FORMAT_B10G10R10A2_UNORM/g s/MESA_FORMAT_Z24_S8\b/MESA_FORMAT_S8_UINT_Z24_UNORM/g s/MESA_FORMAT_S8_Z24\b/MESA_FORMAT_Z24_UNORM_S8_UINT/g s/MESA_FORMAT_X8_Z24\b/MESA_FORMAT_Z24_UNORM_X8_UINT/g s/MESA_FORMAT_Z24_X8\b/MESA_FORMAT_X8Z24_UNORM/g s/MESA_FORMAT_RGB9_E5_FLOAT\b/MESA_FORMAT_R9G9B9E5_FLOAT/g s/MESA_FORMAT_R11_G11_B10_FLOAT\b/MESA_FORMAT_R11G11B10_FLOAT/g s/MESA_FORMAT_Z32_FLOAT_X24S8\b/MESA_FORMAT_Z32_FLOAT_S8X24_UINT/g s/MESA_FORMAT_ABGR2101010_UINT\b/MESA_FORMAT_R10G10B10A2_UINT/g s/MESA_FORMAT_XRGB_UNORM\b/MESA_FORMAT_B4G4R4X4_UNORM/g s/MESA_FORMAT_XRGB1555_UNORM\b/MESA_FORMAT_B5G5R5X1_UNORM/g s/MESA_FORMAT_XRGB2101010_UNORM\b/MESA_FORMAT_B10G10R10X2_UNORM/g s/MESA_FORMAT_AL88\b/MESA_FORMAT_L8A8_UNORM/g s/MESA_FORMAT_AL88_REV\b/MESA_FORMAT_A8L8_UNORM/g s/MESA_FORMAT_AL1616\b/MESA_FORMAT_L16A16_UNORM/g s/MESA_FORMAT_AL1616_REV\b/MESA_FORMAT_A16L16_UNORM/g s/MESA_FORMAT_RG88\b/MESA_FORMAT_G8R8_UNORM/g
Re: [Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification
Thanks Marek, I sent out a new set of patches, otherwise there is a branch called NewStartAMF at fdo:~mmueller/mesa with everything in. Mark On Sun, Jan 26, 2014 at 3:03 PM, Marek Olšák mar...@gmail.com wrote: Mark, I will do it if you send me a git link to your branch which merges cleanly and compiles successfully. Marek On Sun, Jan 26, 2014 at 10:27 PM, Brian Paul brian.e.p...@gmail.com wrote: On Sun, Jan 26, 2014 at 11:45 AM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Jan 24, 2014 at 2:50 AM, Marek Olšák mar...@gmail.com wrote: On Thu, Jan 23, 2014 at 8:36 PM, Kenneth Graunke kenn...@whitecape.org wrote: On 01/23/2014 11:19 AM, Mark Mueller wrote: That works for sRGB, but what about other color spaces that may be added in the future? I'm fine with leaving that to another day and with using the _SRGB decoration, as you say. Oy. Let's not worry about things that hypothetically could be someday. We need to do what makes sense today. Yeah, let's commit this. We can discuss the other things later, including if SRGB should be a type or a modifier. The series is already pretty good. Marek I'm not sure if further acknowledgement is needed to push the series, but, either way, I'm not authorized to push these. The series looks good to me too, but I'd also like to see the _UNORM8 - _SRGB suffix change in a follow-up patch. I tried applying the patch series to my local tree but it doesn't apply cleanly. My recent mesa: add missing ETC2_SRGB cases in formats.c change, and possibly others are at fault. Sorry. If you can rebase the series on top of master I can apply them, unless Marek beats me to it (I'm in the midst of traveling today). -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification
On Thu, Jan 23, 2014 at 2:24 AM, Marek Olšák mar...@gmail.com wrote: On Thu, Jan 23, 2014 at 4:17 AM, Mark Mueller markkmuel...@gmail.com wrote: Hi Merek, I'm Marek. On Wed, Jan 22, 2014 at 2:49 PM, Marek Olšák mar...@gmail.com wrote: Hi Mark, [...] Also, I have a proposal for SRGB formats. MESA_FORMAT_SRGB_UNORM8 and MESA_FORMAT_SA8B8G8R8_UNORM look weird, because they are not really UNORM and there is also no stencil. :) How about this: MESA_FORMAT_RGB_SRGB8 (denoting an array format of the SRGB type and 8 bits per channel) and MESA_FORMAT_A8B8G8R8_SRGB (denoting a packed format of the SRGB type). I agree it could be better. My reasoning for avoiding what you suggested is that it creates an ambiguity between color information and storage type. For instance, sRBG color space values could feasibly be stored as floats, half floats, snorms, or unorms, could they not? Thus the S is a color component I don't think so. An sRGB channel is always a byte. You cannot store both linear and sRGB values in an 8-bit format without losing precision for either one of them, which I think is why there are sRGB formats for sRGB values and other formats for linear values. Bigger texture types do not suffer from this limitation so much. Also, sRGB is not very well defined outside of the range [0, 1], which leaves UNORM as the only suitable type. That works for sRGB, but what about other color spaces that may be added in the future? I'm fine with leaving that to another day and with using the _SRGB decoration, as you say. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V3 PATCH 1/8] mesa: 's/\bgl_format\b/mesa_format/g'. Use better name for Mesa Formats enum
On Fri, Jan 17, 2014 at 8:58 AM, Brian Paul bri...@vmware.com wrote: On 01/16/2014 10:13 PM, Mark Mueller wrote: This series encompases the much discussed specification and renaming of MESA_FORMATs, which now is packed into 8 patches Signed-off-by: Mark Mueller markkmuel...@gmail.com --- Well, our other enum typedefs (and structs) all use the gl_ prefix. But the other enum values don't use MESA_ prefixes so gl_formats are weird that way. I'm kind on the fence about this change. -Brian Obviously it's not critical, but the gl_ prefix is confusing because of the weirdness, and Ken recommended a name change thus I took a stab at it. I've left this change in V4 of the series. Would it be more convincing with a different name, like mgl_formats, or mesa_gl_formats? Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification
Hi Merek, On Wed, Jan 22, 2014 at 2:49 PM, Marek Olšák mar...@gmail.com wrote: Hi Mark, Could you please mention or document somewhere in the code (e.g. in main/formats.h) which _REV formats are incorrect according to you? Sorry if you did so already, I haven't read your other patches yet. There was not a universally correct solution for some of the _REV MESA_FORMATs thus I defaulted to the OGL definition of _REV when eliminating the _REV decoration. For the examples below, MESA_FORMAT's use of _REV sometimes differs from what is described in the OGL pixel transfer doc, which says the following about GL_UNSIGNED_SHORT_5_6_5, for example: This can have a _REV at the end of it. This would mean to reverse the order of the components in the data. In REV mode, the first component, the one that matches the first 5, would go into the last component specified by the format. Using _REV with MESA_FORMATs to have a big and little endian representation of a format agrees with the OGL definition most of the time but not for formats where each color components isn't equally divided on a byte boundary: MESA_FORMAT_RGB565, /* RGGG GGGB */ MESA_FORMAT_RGB565_REV, /* GGGB RGGG */ MESA_FORMAT_ARGB,/* */ MESA_FORMAT_ARGB_REV,/* */ MESA_FORMAT_RGBA5551,/* RGGG GGBB BBBA */ MESA_FORMAT_ARGB1555,/* ARRR RRGG GGGB */ MESA_FORMAT_ARGB1555_REV,/* GGGB ARRR RRGG */ Searching all of Mesa for the above _REV formats reveals few hits within the drivers that are actually employing the endianess interpretation of _REV, vs the OGL interpretation. For example radeon_screen.c: rgbFormat = _mesa_little_endian() ? MESA_FORMAT_RGB565 : MESA_FORMAT_RGB565_REV; vs radeon_pixel_read.c case GL_UNSIGNED_SHORT_5_6_5: return MESA_FORMAT_RGB565; case GL_UNSIGNED_SHORT_5_6_5_REV: return MESA_FORMAT_RGB565_REV; In many of the cases that rely on the endianess interpretation, the _REV decoration is ignored anyway. So, based on your request, I should add a comment above the applicable format_unpack and format pack functions that says this function does not match the current Mesa definition of insert format here. Is that acceptable? With the latest patch set I just buried my head in the sand on this one. Also, I have a proposal for SRGB formats. MESA_FORMAT_SRGB_UNORM8 and MESA_FORMAT_SA8B8G8R8_UNORM look weird, because they are not really UNORM and there is also no stencil. :) How about this: MESA_FORMAT_RGB_SRGB8 (denoting an array format of the SRGB type and 8 bits per channel) and MESA_FORMAT_A8B8G8R8_SRGB (denoting a packed format of the SRGB type). I agree it could be better. My reasoning for avoiding what you suggested is that it creates an ambiguity between color information and storage type. For instance, sRBG color space values could feasibly be stored as floats, half floats, snorms, or unorms, could they not? Thus the S is a color component modifier, not storage type. Would it be too awkward to use M for stencil mask instead of S? Is there a clearer method that doesn't hide storage type information? Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification
This series introduces a specification for 3 types of MESA_FORMAT names - Type A (array formats), Type C (compressed formats), and Type P (packed formats), and then performs a series of substitutions grouped by type. Builds of all default gallium and DRI drivers were verified and no regressions were observed w/piglit tests on the i965 driver. The format_unpack functions were used to verify component orders, except with some _REV formats, which appeared to be incorrect, but not used in normal testing. V4 rearranges a number of A and P types to match current use by format_unpack.c Mark Mueller (7): 1 s/\bgl_format\b/mesa_format/g. Use better name for Mesa Formats enum 2 Change all 4 color component unsigned byte formats to meet spec for P Type formats: s/MESA_FORMAT_RGBA\b/MESA_FORMAT_A8B8G8R8_UNORM/g s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_R8G8B8A8_UNORM/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_B8G8R8A8_UNORM/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A8R8G8B8_UNORM/g s/MESA_FORMAT_RGBX\b/MESA_FORMAT_X8B8G8R8_UNORM/g s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_R8G8B8X8_UNORM/g s/MESA_FORMAT_XRGB\b/MESA_FORMAT_B8G8R8X8_UNORM/g s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_X8R8G8B8_UNORM/g 3 Update comments. Conversion of the following Type A formats: s/MESA_FORMAT_RGB888\b/MESA_FORMAT_BGR_UNORM8/g s/MESA_FORMAT_BGR888\b/MESA_FORMAT_RGB_UNORM8/g s/MESA_FORMAT_A8\b/MESA_FORMAT_A_UNORM8/g s/MESA_FORMAT_A16\b/MESA_FORMAT_A_UNORM16/g s/MESA_FORMAT_L8\b/MESA_FORMAT_L_UNORM8/g s/MESA_FORMAT_L16\b/MESA_FORMAT_L_UNORM16/g s/MESA_FORMAT_I8\b/MESA_FORMAT_I_UNORM8/g s/MESA_FORMAT_I16\b/MESA_FORMAT_I_UNORM16/g s/MESA_FORMAT_R8\b/MESA_FORMAT_R_UNORM8/g s/MESA_FORMAT_R16\b/MESA_FORMAT_R_UNORM16/g s/MESA_FORMAT_Z16\b/MESA_FORMAT_Z_UNORM16/g s/MESA_FORMAT_Z32\b/MESA_FORMAT_Z_UNORM32/g s/MESA_FORMAT_S8\b/MESA_FORMAT_S_UINT8/g s/MESA_FORMAT_SRGB8\b/MESA_FORMAT_SBGR_UNORM8/g s/MESA_FORMAT_RGBA_16\b/MESA_FORMAT_RGBA_UNORM16/g s/MESA_FORMAT_SL8\b/MESA_FORMAT_SL_UNORM8/g s/MESA_FORMAT_Z32_FLOAT\b/MESA_FORMAT_Z_FLOAT32/g s/MESA_FORMAT_XBGR16161616_UNORM\b/MESA_FORMAT_RGBX_UNORM16/g s/MESA_FORMAT_XBGR16161616_SNORM\b/MESA_FORMAT_RGBX_SNORM16/g s/MESA_FORMAT_XBGR16161616_FLOAT\b/MESA_FORMAT_RGBX_FLOAT16/g s/MESA_FORMAT_XBGR16161616_UINT\b/MESA_FORMAT_RGBX_UINT16/g s/MESA_FORMAT_XBGR16161616_SINT\b/MESA_FORMAT_RGBX_SINT16/g s/MESA_FORMAT_XBGR32323232_FLOAT\b/MESA_FORMAT_RGBX_FLOAT32/g s/MESA_FORMAT_XBGR32323232_UINT\b/MESA_FORMAT_RGBX_UINT32/g s/MESA_FORMAT_XBGR32323232_SINT\b/MESA_FORMAT_RGBX_SINT32/g s/MESA_FORMAT_XBGR_UINT\b/MESA_FORMAT_RGBX_UINT8/g s/MESA_FORMAT_XBGR_SINT\b/MESA_FORMAT_RGBX_SINT8/g 4 Conversion of Type P formats as follows (w/related comment fixes): s/MESA_FORMAT_RGB565\b/MESA_FORMAT_B5G6R5_UNORM/g s/MESA_FORMAT_RGB565_REV\b/MESA_FORMAT_R5G6B5_UNORM/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_B4G4R4A4_UNORM/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A4R4G4B4_UNORM/g s/MESA_FORMAT_RGBA5551\b/MESA_FORMAT_A1B5G5R5_UNORM/g s/MESA_FORMAT_XBGR_SNORM\b/MESA_FORMAT_R8G8B8X8_SNORM/g s/MESA_FORMAT_XBGR_SRGB\b/MESA_FORMAT_SR8G8B8X8_UNORM/g s/MESA_FORMAT_ARGB1555\b/MESA_FORMAT_B5G5R5A1_UNORM/g s/MESA_FORMAT_ARGB1555_REV\b/MESA_FORMAT_A1R5G5B5_UNORM/g s/MESA_FORMAT_AL44\b/MESA_FORMAT_L4A4_UNORM/g s/MESA_FORMAT_RGB332\b/MESA_FORMAT_B2G3R3_UNORM/g s/MESA_FORMAT_ARGB2101010\b/MESA_FORMAT_B10G10R10A2_UNORM/g s/MESA_FORMAT_Z24_S8\b/MESA_FORMAT_S8_UINT_Z24_UNORM/g s/MESA_FORMAT_S8_Z24\b/MESA_FORMAT_Z24_UNORM_S8_UINT/g s/MESA_FORMAT_X8_Z24\b/MESA_FORMAT_Z24_UNORM_X8_UINT/g s/MESA_FORMAT_Z24_X8\b/MESA_FORMAT_X8Z24_UNORM/g s/MESA_FORMAT_RGB9_E5_FLOAT\b/MESA_FORMAT_R9G9B9E5_FLOAT/g s/MESA_FORMAT_R11_G11_B10_FLOAT\b/MESA_FORMAT_R11G11B10_FLOAT/g s/MESA_FORMAT_Z32_FLOAT_X24S8\b/MESA_FORMAT_Z32_FLOAT_S8X24_UINT/g s/MESA_FORMAT_ABGR2101010_UINT\b/MESA_FORMAT_R10G10B10A2_UINT/g s/MESA_FORMAT_XRGB_UNORM\b/MESA_FORMAT_B4G4R4X4_UNORM/g s/MESA_FORMAT_XRGB1555_UNORM\b/MESA_FORMAT_B5G5R5X1_UNORM/g s/MESA_FORMAT_XRGB2101010_UNORM\b/MESA_FORMAT_B10G10R10X2_UNORM/g s/MESA_FORMAT_AL88\b/MESA_FORMAT_L8A8_UNORM/g s/MESA_FORMAT_AL88_REV\b/MESA_FORMAT_A8L8_UNORM/g s/MESA_FORMAT_AL1616\b/MESA_FORMAT_L16A16_UNORM/g s/MESA_FORMAT_AL1616_REV\b/MESA_FORMAT_A16L16_UNORM/g s/MESA_FORMAT_RG88\b/MESA_FORMAT_G8R8_UNORM
Re: [Mesa-dev] [V3 PATCH 4/8] mesa: MESA_FORMAT Type P Conversion
On Sun, Jan 19, 2014 at 7:56 PM, Michel Dänzer mic...@daenzer.net wrote: On Fre, 2014-01-17 at 03:47 -0800, Mark Mueller wrote: diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h index 348d2f4..fb43c83 100644 --- a/src/mesa/main/formats.h +++ b/src/mesa/main/formats.h @@ -182,14 +182,14 @@ typedef enum MESA_FORMAT_RGB_UNORM8, /* Type P formats */ - MESA_FORMAT_RGB565, /* RGGG GGGB */ - MESA_FORMAT_RGB565_REV, /* GGGB RGGG */ - MESA_FORMAT_ARGB, /* */ - MESA_FORMAT_ARGB_REV, /* */ - MESA_FORMAT_RGBA5551,/* RGGG GGBB BBBA */ - MESA_FORMAT_ARGB1555, /* ARRR RRGG GGGB */ - MESA_FORMAT_ARGB1555_REV, /* GGGB ARRR RRGG */ - MESA_FORMAT_AL44, /* */ + MESA_FORMAT_B5G6R5_UNORM, /* BGGG GGGR */ + MESA_FORMAT_R5G6B5_UNORM, /* RGGG GGGB */ + MESA_FORMAT_B4G4R4A4_UNORM, /* */ + MESA_FORMAT_A4R4G4B4_UNORM, /* */ + MESA_FORMAT_A1B5G5R5_UNORM, /* ARRR RRGG GGGB */ + MESA_FORMAT_B5G5R5A1_UNORM, /* BGGG GGRR RRRA */ + MESA_FORMAT_A1R5G5B5_UNORM, /* ARRR RRGG GGGB */ + MESA_FORMAT_L4A4_UNORM, /* */ Please keep these comments aligned with the other comments describing packed format layouts. (Please also don't remove the header comments explaining the format of these comments) Sorry, there were some fossilized tabs in there that I _had_ removed in an earlier patch but they came back like the night of the living dead. Also, why are you changing the component order in the comments for these, but not for some other packed formats in the series? Last but not least, there are a few cases in the series where you're defining a format as 'type A', when these comments clearly show that they're packed formats. Please be careful. Unlike the 100's of global substitutions, all of the comments have to be done by hand and there were some stragglers that got through. With the major shift to a lot more P type formats, much of that has changed again. I should have a new patchset revision ready by tomorrow. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [V3 PATCH 7/8] mesa: Rename XBGR MESA_FORMATs
Remove/update related comments and rename MESA_FORMAT_XBGR A Type formats to match naming spec as follows: s/\bMESA_FORMAT_XBGR_SNORM\b/MESA_FORMAT_RGBX_SNORM8/g s/\bMESA_FORMAT_XBGR_SRGB\b/MESA_FORMAT_SRGBX_UNORM8/g s/\bMESA_FORMAT_XBGR_UINT\b/MESA_FORMAT_RGBX_UINT8/g s/\bMESA_FORMAT_XBGR_SINT\b/MESA_FORMAT_RGBX_SINT8/g s/\bMESA_FORMAT_XBGR16161616_UNORM\b/MESA_FORMAT_RGBX_UNORM16/g s/\bMESA_FORMAT_XBGR16161616_SNORM\b/MESA_FORMAT_RGBX_SNORM16/g s/\bMESA_FORMAT_XBGR16161616_FLOAT\b/MESA_FORMAT_RGBX_FLOAT16/g s/\bMESA_FORMAT_XBGR16161616_UINT\b/MESA_FORMAT_RGBX_UINT16/g s/\bMESA_FORMAT_XBGR16161616_SINT\b/MESA_FORMAT_RGBX_SINT16/g s/\bMESA_FORMAT_XBGR32323232_UNORM\b/MESA_FORMAT_RGBX_UNORM32/g s/\bMESA_FORMAT_XBGR32323232_SNORM\b/MESA_FORMAT_RGBX_SNORM32/g s/\bMESA_FORMAT_XBGR32323232_FLOAT\b/MESA_FORMAT_RGBX_FLOAT32/g s/\bMESA_FORMAT_XBGR32323232_UINT\b/MESA_FORMAT_RGBX_UINT32/g s/\bMESA_FORMAT_XBGR32323232_SINT\b/MESA_FORMAT_RGBX_SINT32/g Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_surface_formats.c | 24 +++--- src/mesa/main/format_pack.c | 60 +++--- src/mesa/main/format_unpack.c | 36 - src/mesa/main/formats.c | 100 src/mesa/main/formats.h | 24 +++--- src/mesa/main/texformat.c | 20 ++--- src/mesa/main/texstore.c| 70 - src/mesa/state_tracker/st_format.c | 48 ++-- src/mesa/swrast/s_texfetch.c| 24 +++--- 9 files changed, 203 insertions(+), 203 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c index 2620131..f7afce2 100644 --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c @@ -507,19 +507,19 @@ brw_format_for_mesa_format(mesa_format mesa_format) [MESA_FORMAT_B4G4R4X4_UNORM] = 0, [MESA_FORMAT_B5G5R5X1_UNORM] = BRW_SURFACEFORMAT_B5G5R5X1_UNORM, - [MESA_FORMAT_XBGR_SNORM] = 0, - [MESA_FORMAT_XBGR_SRGB] = 0, - [MESA_FORMAT_XBGR_UINT] = 0, - [MESA_FORMAT_XBGR_SINT] = 0, + [MESA_FORMAT_RGBX_SNORM8] = 0, + [MESA_FORMAT_SRGBX_UNORM8] = 0, + [MESA_FORMAT_RGBX_UINT8] = 0, + [MESA_FORMAT_RGBX_SINT8] = 0, [MESA_FORMAT_B10G10R10X2_UNORM] = BRW_SURFACEFORMAT_B10G10R10X2_UNORM, - [MESA_FORMAT_XBGR16161616_UNORM] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM, - [MESA_FORMAT_XBGR16161616_SNORM] = 0, - [MESA_FORMAT_XBGR16161616_FLOAT] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT, - [MESA_FORMAT_XBGR16161616_UINT] = 0, - [MESA_FORMAT_XBGR16161616_SINT] = 0, - [MESA_FORMAT_XBGR32323232_FLOAT] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT, - [MESA_FORMAT_XBGR32323232_UINT] = 0, - [MESA_FORMAT_XBGR32323232_SINT] = 0, + [MESA_FORMAT_RGBX_UNORM16] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM, + [MESA_FORMAT_RGBX_SNORM16] = 0, + [MESA_FORMAT_RGBX_FLOAT16] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT, + [MESA_FORMAT_RGBX_UINT16] = 0, + [MESA_FORMAT_RGBX_SINT16] = 0, + [MESA_FORMAT_RGBX_FLOAT32] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT, + [MESA_FORMAT_RGBX_UINT32] = 0, + [MESA_FORMAT_RGBX_SINT32] = 0, }; assert(mesa_format MESA_FORMAT_COUNT); return table[mesa_format]; diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c index d80e25c..e604cd3 100644 --- a/src/mesa/main/format_pack.c +++ b/src/mesa/main/format_pack.c @@ -1711,7 +1711,7 @@ pack_float_XRGB1555_UNORM(const GLfloat src[4], void *dst) /* - * MESA_FORMAT_XBGR_SNORM + * MESA_FORMAT_RGBX_SNORM8 */ static void @@ -1726,7 +1726,7 @@ pack_float_XBGR_SNORM(const GLfloat src[4], void *dst) /* - * MESA_FORMAT_XBGR_SRGB + * MESA_FORMAT_SRGBX_UNORM8 */ static void @@ -1764,7 +1764,7 @@ pack_float_XRGB2101010_UNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_UNORM */ +/* MESA_FORMAT_RGBX_UNORM16 */ static void pack_ubyte_XBGR16161616_UNORM(const GLubyte src[4], void *dst) @@ -1787,7 +1787,7 @@ pack_float_XBGR16161616_UNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_SNORM */ +/* MESA_FORMAT_RGBX_SNORM16 */ static void pack_float_XBGR16161616_SNORM(const GLfloat src[4], void *dst) @@ -1800,7 +1800,7 @@ pack_float_XBGR16161616_SNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_FLOAT */ +/* MESA_FORMAT_RGBX_FLOAT16 */ static void pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void *dst) @@ -1812,7 +1812,7 @@ pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void *dst) d[3] = _mesa_float_to_half(1.0); } -/* MESA_FORMAT_XBGR32323232_FLOAT */ +/* MESA_FORMAT_RGBX_FLOAT32 */ static void pack_float_XBGR32323232_FLOAT(const GLfloat src[4], void *dst
Re: [Mesa-dev] [V3 PATCH 0/8] mesa: Naming MESA_FORMATs to a specification
On Fri, Jan 17, 2014 at 2:43 AM, Erik Faye-Lund kusmab...@gmail.com wrote: On Fri, Jan 17, 2014 at 9:22 AM, Christian König deathsim...@vodafone.de wrote: But as a general note on writing patches: Please limit the subject line to a sane length! Something between 60 and 80 chars should be ok. Indeed. The regex patterns are nice to have. But please, put them in the *body* of the commit message. Ugh. A combination of PEBKAC and using a new editor. Sorry for the substantial noise. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V3 PATCH 2/8] mesa: Change 4 color component ubyte formats
On Fri, Jan 17, 2014 at 8:58 AM, Brian Paul bri...@vmware.com wrote: On 01/17/2014 03:45 AM, Mark Mueller wrote: Change all 4 color component unsigned byte formats to meet spec: s/MESA_FORMAT_RGBA\b/MESA_FORMAT_ABGR_UNORM8/g s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_RGBA_UNORM8/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_BGRA_UNORM8/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_ARGB_UNORM8/g s/MESA_FORMAT_RGBX\b/MESA_FORMAT_XBGR_UNORM8/g s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_RGBX_UNORM8/g s/MESA_FORMAT_XRGB\b/MESA_FORMAT_BGRX_UNORM8/g s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_XRGB_UNORM8/g I'm not sure this is right. If you look at the existing code such as src/mesa/main/format_{un}pack.c you'll see that these formats are treated as packed formats, not arrays. Ah. Array formats are really rare with OGL, that was unexpected but now really ancient issues with memory throughput optimization are surfacing. Those were the days. Thus Array Types would only include the much smaller group of all 32 bit-per-component formats, and formats with an odd number of 8 or 16 bit components. Right? So the naming convention would be a derivation of MESA_FORMAT_R8G8B8A8_UNORM for these. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [V3 PATCH 2/8] mesa: Change 4 color component ubyte formats
On Fri, Jan 17, 2014 at 1:24 PM, Marek Olšák mar...@gmail.com wrote: Incorrect. You have to manually check if the pack and unpack functions access the components using bitwise operations or arrays. Consider char pix[]. The array formats use arrays for accessing, for example: char *p = pix[(y*width+x)*4]; p[0] = r; p[1] = g; p[2] = b; p[3] = a; Packed formats use bitwise operators for accessing, for example: uint32_t *p = pix[(y*width+x)*4]; *p = r | (g 8) | (b 16) | (a 24); Hang on, that's precisely what I did and when I tallied up the results from the manual inspection, the rule I provided below fit. For example, in format_pack.c, any pack_ubyte function that does not use a PACK_COLOR_ macro is either 32 bits per component, or an odd number of components with 8 or 16 bits: MESA_FORMAT_R_UNORM8, MESA_FORMAT_RGB_UNORM8. I admit that I messed one, which is 16 bit floats - those are arrays too. It's okay to have both RGBA8 array and packed formats. For example, Gallium has these array formats: PIPE_FORMAT_R8G8B8A8_UNORM PIPE_FORMAT_B8G8R8A8_UNORM PIPE_FORMAT_A8R8G8B8_UNORM PIPE_FORMAT_A8B8G8R8_UNORM And it defines these packed formats by using the array formats according to the CPU architecture: #if defined(PIPE_ARCH_LITTLE_ENDIAN) #define PIPE_FORMAT_RGBA_UNORM PIPE_FORMAT_R8G8B8A8_UNORM #define PIPE_FORMAT_BGRA_UNORM PIPE_FORMAT_B8G8R8A8_UNORM #define PIPE_FORMAT_ARGB_UNORM PIPE_FORMAT_A8R8G8B8_UNORM #define PIPE_FORMAT_ABGR_UNORM PIPE_FORMAT_A8B8G8R8_UNORM #elif defined(PIPE_ARCH_BIG_ENDIAN) #define PIPE_FORMAT_ABGR_UNORM PIPE_FORMAT_R8G8B8A8_UNORM #define PIPE_FORMAT_ARGB_UNORM PIPE_FORMAT_B8G8R8A8_UNORM #define PIPE_FORMAT_BGRA_UNORM PIPE_FORMAT_A8R8G8B8_UNORM #define PIPE_FORMAT_RGBA_UNORM PIPE_FORMAT_A8B8G8R8_UNORM #endif This way, Gallium has all the possible RGBA8 array formats and also the possible RGBA8 packed formats, so we can use whatever we want. The MESA_FORMATs are used to literally tag the application's Internal formats such that the driver can better know the application's intention (admittedly I'm also looking beyond _mesa_choose_tex_format, but that is for another time). The Array Type formats were proposed to indicate that the component order is independent of endianess, whereas Packed Type component orders _do_ depend on endianess. Acknowledging these Types is an attempt to eradicate the confusion. I have no input about mixing types within Gallium, but within Mesa my preference is to adhere to that distinction. I find Francisco's method to be less confusing then Gallium's (not just because of the helpful comment). Here it is with P Type formats: /* * Define endian-invariant aliases for some mesa formats that are * defined in terms of their channel layout from LSB to MSB in a * 32-bit word. The actual byte offsets matter here because the user * is allowed to bit-cast one format into another and get predictable * results. */ #ifdef MESA_BIG_ENDIAN # define MESA_FORMAT_RGBA_8 MESA_FORMAT_A8B8G8R8_UNORM # define MESA_FORMAT_RG_16 MESA_FORMAT_G16R16_UNORM # define MESA_FORMAT_RG_8 MESA_FORMAT_G8R8_UNORM # define MESA_FORMAT_SIGNED_RGBA_8 MESA_FORMAT_A8B8G8R8_SNORM # define MESA_FORMAT_SIGNED_RG_16 MESA_FORMAT_G16R16_SNORM # define MESA_FORMAT_SIGNED_RG_8 MESA_FORMAT_G8R8_SNORM #else # define MESA_FORMAT_RGBA_8 MESA_FORMAT_R8G8B8A8_UNORM # define MESA_FORMAT_RG_16 MESA_FORMAT_R16G16_UNORM # define MESA_FORMAT_RG_8 MESA_FORMAT_R8G8_UNORM # define MESA_FORMAT_SIGNED_RGBA_8 MESA_FORMAT_R8G8B8A8_SNORM # define MESA_FORMAT_SIGNED_RG_16 MESA_FORMAT_R16G16_SNORM # define MESA_FORMAT_SIGNED_RG_8 MESA_FORMAT_R8G8_SNORM #endif Mark Marek On Fri, Jan 17, 2014 at 9:41 PM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Jan 17, 2014 at 8:58 AM, Brian Paul bri...@vmware.com wrote: On 01/17/2014 03:45 AM, Mark Mueller wrote: Change all 4 color component unsigned byte formats to meet spec: s/MESA_FORMAT_RGBA\b/MESA_FORMAT_ABGR_UNORM8/g s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_RGBA_UNORM8/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_BGRA_UNORM8/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_ARGB_UNORM8/g s/MESA_FORMAT_RGBX\b/MESA_FORMAT_XBGR_UNORM8/g s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_RGBX_UNORM8/g s/MESA_FORMAT_XRGB\b/MESA_FORMAT_BGRX_UNORM8/g s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_XRGB_UNORM8/g I'm not sure this is right. If you look at the existing code such as src/mesa/main/format_{un}pack.c you'll see that these formats are treated as packed formats, not arrays. Ah. Array formats are really rare with OGL, that was unexpected but now really ancient issues with memory throughput optimization are surfacing. Those were the days. Thus Array Types would only include the much smaller group of all 32 bit-per-component formats, and formats with an odd number of 8 or 16 bit components. Right? So the naming convention
[Mesa-dev] [V3 PATCH 7/8] mesa: Remove/update related comments and rename MESA_FORMAT_XBGR A Type MESA_FORMATs to match naming spec as follows: s/\bMESA_FORMAT_XBGR8888_SNORM\b/MESA_FORMAT_RGBX_SNORM8
Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_surface_formats.c | 24 +++--- src/mesa/main/format_pack.c | 60 +++--- src/mesa/main/format_unpack.c | 36 - src/mesa/main/formats.c | 100 src/mesa/main/formats.h | 24 +++--- src/mesa/main/texformat.c | 20 ++--- src/mesa/main/texstore.c| 70 - src/mesa/state_tracker/st_format.c | 48 ++-- src/mesa/swrast/s_texfetch.c| 24 +++--- 9 files changed, 203 insertions(+), 203 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c index 2620131..f7afce2 100644 --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c @@ -507,19 +507,19 @@ brw_format_for_mesa_format(mesa_format mesa_format) [MESA_FORMAT_B4G4R4X4_UNORM] = 0, [MESA_FORMAT_B5G5R5X1_UNORM] = BRW_SURFACEFORMAT_B5G5R5X1_UNORM, - [MESA_FORMAT_XBGR_SNORM] = 0, - [MESA_FORMAT_XBGR_SRGB] = 0, - [MESA_FORMAT_XBGR_UINT] = 0, - [MESA_FORMAT_XBGR_SINT] = 0, + [MESA_FORMAT_RGBX_SNORM8] = 0, + [MESA_FORMAT_SRGBX_UNORM8] = 0, + [MESA_FORMAT_RGBX_UINT8] = 0, + [MESA_FORMAT_RGBX_SINT8] = 0, [MESA_FORMAT_B10G10R10X2_UNORM] = BRW_SURFACEFORMAT_B10G10R10X2_UNORM, - [MESA_FORMAT_XBGR16161616_UNORM] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM, - [MESA_FORMAT_XBGR16161616_SNORM] = 0, - [MESA_FORMAT_XBGR16161616_FLOAT] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT, - [MESA_FORMAT_XBGR16161616_UINT] = 0, - [MESA_FORMAT_XBGR16161616_SINT] = 0, - [MESA_FORMAT_XBGR32323232_FLOAT] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT, - [MESA_FORMAT_XBGR32323232_UINT] = 0, - [MESA_FORMAT_XBGR32323232_SINT] = 0, + [MESA_FORMAT_RGBX_UNORM16] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM, + [MESA_FORMAT_RGBX_SNORM16] = 0, + [MESA_FORMAT_RGBX_FLOAT16] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT, + [MESA_FORMAT_RGBX_UINT16] = 0, + [MESA_FORMAT_RGBX_SINT16] = 0, + [MESA_FORMAT_RGBX_FLOAT32] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT, + [MESA_FORMAT_RGBX_UINT32] = 0, + [MESA_FORMAT_RGBX_SINT32] = 0, }; assert(mesa_format MESA_FORMAT_COUNT); return table[mesa_format]; diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c index d80e25c..e604cd3 100644 --- a/src/mesa/main/format_pack.c +++ b/src/mesa/main/format_pack.c @@ -1711,7 +1711,7 @@ pack_float_XRGB1555_UNORM(const GLfloat src[4], void *dst) /* - * MESA_FORMAT_XBGR_SNORM + * MESA_FORMAT_RGBX_SNORM8 */ static void @@ -1726,7 +1726,7 @@ pack_float_XBGR_SNORM(const GLfloat src[4], void *dst) /* - * MESA_FORMAT_XBGR_SRGB + * MESA_FORMAT_SRGBX_UNORM8 */ static void @@ -1764,7 +1764,7 @@ pack_float_XRGB2101010_UNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_UNORM */ +/* MESA_FORMAT_RGBX_UNORM16 */ static void pack_ubyte_XBGR16161616_UNORM(const GLubyte src[4], void *dst) @@ -1787,7 +1787,7 @@ pack_float_XBGR16161616_UNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_SNORM */ +/* MESA_FORMAT_RGBX_SNORM16 */ static void pack_float_XBGR16161616_SNORM(const GLfloat src[4], void *dst) @@ -1800,7 +1800,7 @@ pack_float_XBGR16161616_SNORM(const GLfloat src[4], void *dst) } -/* MESA_FORMAT_XBGR16161616_FLOAT */ +/* MESA_FORMAT_RGBX_FLOAT16 */ static void pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void *dst) @@ -1812,7 +1812,7 @@ pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void *dst) d[3] = _mesa_float_to_half(1.0); } -/* MESA_FORMAT_XBGR32323232_FLOAT */ +/* MESA_FORMAT_RGBX_FLOAT32 */ static void pack_float_XBGR32323232_FLOAT(const GLfloat src[4], void *dst) @@ -2014,19 +2014,19 @@ _mesa_get_pack_ubyte_rgba_function(mesa_format format) table[MESA_FORMAT_B4G4R4X4_UNORM] = pack_ubyte_XRGB_UNORM; table[MESA_FORMAT_B5G5R5X1_UNORM] = pack_ubyte_XRGB1555_UNORM; - table[MESA_FORMAT_XBGR_SNORM] = NULL; - table[MESA_FORMAT_XBGR_SRGB] = NULL; - table[MESA_FORMAT_XBGR_UINT] = NULL; - table[MESA_FORMAT_XBGR_SINT] = NULL; + table[MESA_FORMAT_RGBX_SNORM8] = NULL; + table[MESA_FORMAT_SRGBX_UNORM8] = NULL; + table[MESA_FORMAT_RGBX_UINT8] = NULL; + table[MESA_FORMAT_RGBX_SINT8] = NULL; table[MESA_FORMAT_B10G10R10X2_UNORM] = pack_ubyte_XRGB2101010_UNORM; - table[MESA_FORMAT_XBGR16161616_UNORM] = pack_ubyte_XBGR16161616_UNORM; - table[MESA_FORMAT_XBGR16161616_SNORM] = NULL; - table[MESA_FORMAT_XBGR16161616_FLOAT] = NULL; - table[MESA_FORMAT_XBGR16161616_UINT] = NULL; - table[MESA_FORMAT_XBGR16161616_SINT] = NULL; - table
[Mesa-dev] [V3 PATCH 0/8] mesa: Naming MESA_FORMATs to a specification
This series introduces a specification for 3 types of MESA_FORMAT names - Type A (array formats), Type C (compressed formats), and Type P (packed formats), and then performs a series of substitutions grouped by type. Builds of all default gallium and DRI drivers were verified and no regressions were observed w/piglit tests on the i965 driver. The format_unpack functions were used to verify component orders, except with some _REV formats, which appeared to be incorrect, but not used in normal testing. Mark Mueller (8): 1 's/\bgl_format\b/mesa_format/g'. Use better name for Mesa Formats enum 2 Change all 4 color component unsigned byte formats to meet spec: s/MESA_FORMAT_RGBA\b/MESA_FORMAT_ABGR_UNORM8/g s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_RGBA_UNORM8/g s/MESA_FORMAT_ARGB\b/MESA_FORMAT_BGRA_UNORM8/g s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_ARGB_UNORM8/g s/MESA_FORMAT_RGBX\b/MESA_FORMAT_XBGR_UNORM8/g s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_RGBX_UNORM8/g s/MESA_FORMAT_XRGB\b/MESA_FORMAT_BGRX_UNORM8/g s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_XRGB_UNORM8/g 3 Update comments. Conversion of the following Type A formats: s/MESA_FORMAT_RGB888\b/MESA_FORMAT_BGR_UNORM8/g s/MESA_FORMAT_BGR888\b/MESA_FORMAT_RGB_UNORM8/g s/MESA_FORMAT_AL88\b/MESA_FORMAT_LA_UNORM8/g s/MESA_FORMAT_AL88_REV\b/MESA_FORMAT_AL_UNORM8/g s/MESA_FORMAT_AL1616\b/MESA_FORMAT_LA_UNORM16/g s/MESA_FORMAT_AL1616_REV\b/MESA_FORMAT_AL_UNORM16/g s/MESA_FORMAT_A8\b/MESA_FORMAT_A_UNORM8/g s/MESA_FORMAT_A16\b/MESA_FORMAT_A_UNORM16/g s/MESA_FORMAT_L8\b/MESA_FORMAT_L_UNORM8/g s/MESA_FORMAT_L16\b/MESA_FORMAT_L_UNORM16/g s/MESA_FORMAT_I8\b/MESA_FORMAT_I_UNORM8/g s/MESA_FORMAT_I16\b/MESA_FORMAT_I_UNORM16/g s/MESA_FORMAT_R8\b/MESA_FORMAT_R_UNORM8/g s/MESA_FORMAT_GR88\b/MESA_FORMAT_RG_UNORM8/g s/MESA_FORMAT_RG88\b/MESA_FORMAT_GR_UNORM8/g s/MESA_FORMAT_R16\b/MESA_FORMAT_R_UNORM16/g s/MESA_FORMAT_GR1616\b/MESA_FORMAT_RG_UNORM16/g s/MESA_FORMAT_RG1616\b/MESA_FORMAT_GR_UNORM16/g s/MESA_FORMAT_Z16\b/MESA_FORMAT_Z_UNORM16/g s/MESA_FORMAT_Z32\b/MESA_FORMAT_Z_UNORM32/g s/MESA_FORMAT_S8\b/MESA_FORMAT_S_UINT8/g s/MESA_FORMAT_SRGBA8\b/MESA_FORMAT_SABGR_UNORM8/g s/MESA_FORMAT_SRGB8\b/MESA_FORMAT_SBGR_UNORM8/g s/MESA_FORMAT_SARGB8\b/MESA_FORMAT_SBGRA_UNORM8/g s/MESA_FORMAT_SL8\b/MESA_FORMAT_SL_UNORM8/g s/MESA_FORMAT_SLA8\b/MESA_FORMAT_SLA_UNORM8/g s/MESA_FORMAT_Z32_FLOAT\b/MESA_FORMAT_Z_FLOAT32/g 4 Conversion of Type P formats as follows (w/related comment fixes): s/\bMESA_FORMAT_RGB565\b/MESA_FORMAT_B5G6R5_UNORM/g s/\bMESA_FORMAT_RGB565_REV\b/MESA_FORMAT_R5G6B5_UNORM/g s/\bMESA_FORMAT_ARGB\b/MESA_FORMAT_B4G4R4A4_UNORM/g s/\bMESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A4R4G4B4_UNORM/g s/\bMESA_FORMAT_RGBA5551\b/MESA_FORMAT_A1B5G5R5_UNORM/g s/\bMESA_FORMAT_ARGB1555\b/MESA_FORMAT_B5G5R5A1_UNORM/g s/\bMESA_FORMAT_ARGB1555_REV\b/MESA_FORMAT_A1R5G5B5_UNORM/g s/\bMESA_FORMAT_AL44\b/MESA_FORMAT_L4A4_UNORM/g s/\bMESA_FORMAT_RGB332\b/MESA_FORMAT_B2G3R3_UNORM/g s/\bMESA_FORMAT_ARGB2101010\b/MESA_FORMAT_B10G10R10A2_UNORM/g s/\bMESA_FORMAT_Z24_S8\b/MESA_FORMAT_S8_UINT_Z24_UNORM/g s/\bMESA_FORMAT_S8_Z24\b/MESA_FORMAT_Z24_UNORM_S8_UINT/g s/\bMESA_FORMAT_X8_Z24\b/MESA_FORMAT_Z24_UNORM_X8_UINT/g s/\bMESA_FORMAT_Z24_X8\b/MESA_FORMAT_X8Z24_UNORM/g s/\bMESA_FORMAT_RGB9_E5_FLOAT\b/MESA_FORMAT_R9G9B9E5_FLOAT/g s/\bMESA_FORMAT_R11_G11_B10_FLOAT\b/MESA_FORMAT_R11G11B10_FLOAT/g s/\bMESA_FORMAT_Z32_FLOAT_X24S8\b/MESA_FORMAT_Z32_FLOAT_S8X24_UINT/g s/\bMESA_FORMAT_ABGR2101010_UINT\b/MESA_FORMAT_R10G10B10A2_UINT/g s/\bMESA_FORMAT_XRGB_UNORM\b/MESA_FORMAT_B4G4R4X4_UNORM/g s/\bMESA_FORMAT_XRGB1555_UNORM\b/MESA_FORMAT_B5G5R5X1_UNORM/g s/\bMESA_FORMAT_XRGB2101010_UNORM\b/MESA_FORMAT_B10G10R10X2_UNORM/g 5 Compressed spelled out color components ALPHA, INTENSITY, and LUMINANCE to A, I, and L: s/\bMESA_FORMAT_ALPHA_UINT8\b/MESA_FORMAT_A_UINT8/g s/\bMESA_FORMAT_ALPHA_UINT16\b/MESA_FORMAT_A_UINT16/g s/\bMESA_FORMAT_ALPHA_UINT32\b/MESA_FORMAT_A_UINT32/g s/\bMESA_FORMAT_ALPHA_INT32\b/MESA_FORMAT_A_INT32/g s/\bMESA_FORMAT_ALPHA_INT16\b/MESA_FORMAT_A_INT16/g s/\bMESA_FORMAT_ALPHA_INT8\b/MESA_FORMAT_A_INT8/g s/\bMESA_FORMAT_INTESITY_UINT8\b/MESA_FORMAT_I_UINT8/g s/\bMESA_FORMAT_INTESITY_UINT16\b/MESA_FORMAT_I_UINT16/g s/\bMESA_FORMAT_INTESITY_UINT32\b/MESA_FORMAT_I_UINT32/g s/\bMESA_FORMAT_INTESITY_INT32\b/MESA_FORMAT_I_INT32/g s/\bMESA_FORMAT_INTESITY_INT16\b/MESA_FORMAT_I_INT16/g s/\bMESA_FORMAT_INTESITY_INT8\b
Re: [Mesa-dev] [PATCH] mesa: rename MESA format names to have the same names as PIPE formats
The predominant feedback on this adventure has been to make the MESA_FORMATs match the PIPE, or gallium formats but every journey I've made down that path has been fraught with peril. There are some cases where PIPE_FORMATs are even more confusing then MESA_FORMATs*. Either there is something that I am missing, or there are things about the PIPE_FORMATS that people aren't aware of, so let me pull out some specific references. The first problem is that in PIPE_FORMATS there is no distinction between array and packed formats, and this has proven to be a big cause for format ambiguity that some are wanting to see addressed. One proposed solution is to represent array formats like (hypothetically): R8G8B8A8; and packed formats as RGBA_ (or vice versa) in the MESA_FORMATs and subsequently modifying the PIPE_FORMATs to suit. But that makes RGBA_1010102 kinda messy (though it could be RGBA_aaa2). So then how to handle this: So how about a poll! Isn't that the rage these days? Please vote on: 1) Should MESA_FORMAT names clearly distinguish between array and packed formats, yes or no? 2) What is your preference for array format naming convention: a) RGBA_UNORM b) R8G8B8A8_UNORM c) RGBA_UNORM8 3) What is your preference for packed format naming convention: a) RGBA5551_UNORM b) R5G5B5A1_UNORM 4) What is your preference for naming packed formats with 10 or more bits: a) RGBA1010102_UNORM b) R10G10B10A2_UNORM c) RGBAaaa2_UNORM d) Croque-monsieur Please vote just once! Thanks, Mark * code snip-it from p_format.h #if defined(PIPE_ARCH_LITTLE_ENDIAN) #define PIPE_FORMAT_RGBA_UNORM PIPE_FORMAT_R8G8B8A8_UNORM #define PIPE_FORMAT_RGBX_UNORM PIPE_FORMAT_R8G8B8X8_UNORM #define PIPE_FORMAT_BGRA_UNORM PIPE_FORMAT_B8G8R8A8_UNORM #define PIPE_FORMAT_BGRX_UNORM PIPE_FORMAT_B8G8R8X8_UNORM #define PIPE_FORMAT_ARGB_UNORM PIPE_FORMAT_A8R8G8B8_UNORM #define PIPE_FORMAT_XRGB_UNORM PIPE_FORMAT_X8R8G8B8_UNORM #define PIPE_FORMAT_ABGR_UNORM PIPE_FORMAT_A8B8G8R8_UNORM #define PIPE_FORMAT_XBGR_UNORM PIPE_FORMAT_X8B8G8R8_UNORM #elif defined(PIPE_ARCH_BIG_ENDIAN) #define PIPE_FORMAT_ABGR_UNORM PIPE_FORMAT_R8G8B8A8_UNORM #define PIPE_FORMAT_XBGR_UNORM PIPE_FORMAT_R8G8B8X8_UNORM #define PIPE_FORMAT_XRGB_UNORM PIPE_FORMAT_B8G8R8X8_UNORM #define PIPE_FORMAT_ARGB_UNORM PIPE_FORMAT_B8G8R8A8_UNORM #define PIPE_FORMAT_XRGB_UNORM PIPE_FORMAT_B8G8R8X8_UNORM #define PIPE_FORMAT_BGRA_UNORM PIPE_FORMAT_A8R8G8B8_UNORM #define PIPE_FORMAT_BGRX_UNORM PIPE_FORMAT_X8R8G8B8_UNORM #define PIPE_FORMAT_RGBA_UNORM PIPE_FORMAT_A8B8G8R8_UNORM #define PIPE_FORMAT_RGBX_UNORM PIPE_FORMAT_X8B8G8R8_UNORM #endif There is more below the surface here too because the above macros aren't employed consistently throughout the gallium drivers as best as I can tell. On Thu, Dec 26, 2013 at 7:48 PM, Mark Mueller markkmuel...@gmail.comwrote: On Thu, Dec 26, 2013 at 6:57 PM, Michel Dänzer mic...@daenzer.net wrote: On Mit, 2013-12-25 at 20:35 -0800, Mark Mueller wrote: On Wed, Dec 25, 2013 at 7:25 PM, Michel Dänzer mic...@daenzer.net wrote: On Mit, 2013-12-25 at 15:19 -0800, Mark Mueller wrote: --Format Base Type P: Packed -- MESA_FORMAT_[[component list,bit width][storage type*][_]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_R5G6B5_UNORM /* RGGG GGGB */ MESA_FORMAT_B4G4R4X4_UNORM /* */ This is slightly confusing in that the PIPE_FORMATs use this convention for naming the components of 'array' formats, packed formats use BGRX (just like packed MESA_FORMATs do now). Beware that not all PIPE_FORMATs have been updated yet according to that distinction. Is this what you are suggesting then? --Format Base Type P: Packed -- MESA_FORMAT_[[component list][bit width per component]_[storage type*]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_RGB565_UNORM /* RGGG GGGB */ MESA_FORMAT_BGRX_UNORM /* */ MESA_FORMAT_Z32_FLOAT_SX824_UINT MESA_FORMAT_RGBA1010102_UINT MESA_FORMAT_RGBE9995_FLOAT That would be more consistent with the current PIPE_FORMAT naming convention, but some of the component size sequences look a bit weird, your original proposal actually makes more sense for those... Sorry, I'm not sure which original proposal you are referring to, could you be more specific? My concern about this latest iteration is that the delimiter
Re: [Mesa-dev] [PATCH V3 02/13] glapi: add plumbing for GL_ARB_draw_indirect and GL_ARB_multi_draw_indirect
As a precautionary note, one using auto tools may want to remove auto-generated source files with git clean -dxf when switching to or from an old branch with this change to make sure the auto-generated source files are regenerated. None of the existing make clean targets remove the autogenerated source or related cache files. Otherwise build errors could result. Mark On Tue, Nov 19, 2013 at 11:27 AM, Ian Romanick i...@freedesktop.org wrote: On 11/09/2013 01:02 AM, Chris Forbes wrote: Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series. Signed-off-by: Chris Forbes chr...@ijw.co.nz I assume 'make check' actually passes? :) With the change below, Reviewed-by: Ian Romanick ian.d.roman...@intel.com. --- src/mapi/glapi/gen/ARB_draw_indirect.xml | 45 src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml| 4 ++- src/mesa/main/tests/dispatch_sanity.cpp | 8 +++--- src/mesa/vbo/vbo_exec_array.c| 32 +++ 5 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 src/mapi/glapi/gen/ARB_draw_indirect.xml diff --git a/src/mapi/glapi/gen/ARB_draw_indirect.xml b/src/mapi/glapi/gen/ARB_draw_indirect.xml new file mode 100644 index 000..7de03cd --- /dev/null +++ b/src/mapi/glapi/gen/ARB_draw_indirect.xml @@ -0,0 +1,45 @@ +?xml version=1.0? +!DOCTYPE OpenGLAPI SYSTEM gl_API.dtd + +OpenGLAPI + +category name=GL_ARB_draw_indirect number=87 + +enum name=DRAW_INDIRECT_BUFFER value=0x8F3F/ +enum name=DRAW_INDIRECT_BUFFER_BINDING value=0x8F43/ + +function name=DrawArraysIndirect offset=assign exec=dynamic +param name=mode type=GLenum/ +param name=indirect type=const GLvoid */ +/function + +function name=DrawElementsIndirect offset=assign exec=dynamic +param name=mode type=GLenum/ +param name=type type=GLenum/ +param name=indirect type=const GLvoid */ +/function + +/category + + +category name=GL_ARB_multi_draw_indirect number=133 + +function name=MultiDrawArraysIndirect offset=assign exec=dynamic +param name=mode type=GLenum/ +param name=indirect type=const GLvoid */ +param name=primcount type=GLsizei/ +param name=stride type=GLsizei/ +/function + +function name=MultiDrawElementsIndirect offset=assign exec=dynamic +param name=mode type=GLenum/ +param name=type type=GLenum/ +param name=indirect type=const GLvoid */ +param name=primcount type=GLsizei/ +param name=stride type=GLsizei/ +/function + +/category + + +/OpenGLAPI diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index 476d943..7af769a 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -98,6 +98,7 @@ API_XML = \ ARB_draw_buffers.xml \ ARB_draw_buffers_blend.xml \ ARB_draw_elements_base_vertex.xml \ + ARB_draw_indirect.xml \ ARB_draw_instanced.xml \ ARB_ES2_compatibility.xml \ ARB_ES3_compatibility.xml \ diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index a2d914a..5c877aa 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -8241,6 +8241,8 @@ !-- ARB extensions #86...#93 -- +xi:include href=ARB_draw_indirect.xml xmlns:xi= http://www.w3.org/2001/XInclude/ + category name=GL_ARB_transform_feedback3 number=94 enum name=MAX_TRANSFORM_FEEDBACK_BUFFERS value=0x8E70/ enum name=MAX_VERTEX_STREAMS value=0x8E71/ @@ -8470,7 +8472,7 @@ xi:include href=ARB_invalidate_subdata.xml xmlns:xi= http://www.w3.org/2001/XInclude/ -!-- ARB extensions #133...#138 -- +!-- ARB extensions #134...#138 -- xi:include href=ARB_texture_buffer_range.xml xmlns:xi= http://www.w3.org/2001/XInclude/ diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp index 922f0ac..e57fb52 100644 --- a/src/mesa/main/tests/dispatch_sanity.cpp +++ b/src/mesa/main/tests/dispatch_sanity.cpp @@ -667,8 +667,8 @@ const struct function gl_core_functions_possible[] = { { glVertexAttribP3uiv, 43, -1 }, { glVertexAttribP4ui, 43, -1 }, { glVertexAttribP4uiv, 43, -1 }, -// { glDrawArraysIndirect, 43, -1 }, // XXX: Add to xml -// { glDrawElementsIndirect, 43, -1 },// XXX: Add to xml + { glDrawArraysIndirect, 43, -1 }, + { glDrawElementsIndirect, 43, -1 }, // { glUniform1d, 43, -1 }, // XXX: Add to xml // { glUniform2d, 43, -1 }, // XXX: Add to xml // { glUniform3d, 43, -1 }, // XXX: Add to xml @@ -877,8 +877,8 @@ const struct
Re: [Mesa-dev] [PATCH] mesa: rename MESA format names to have the same names as PIPE formats
On Thu, Dec 26, 2013 at 6:57 PM, Michel Dänzer mic...@daenzer.net wrote: On Mit, 2013-12-25 at 20:35 -0800, Mark Mueller wrote: On Wed, Dec 25, 2013 at 7:25 PM, Michel Dänzer mic...@daenzer.net wrote: On Mit, 2013-12-25 at 15:19 -0800, Mark Mueller wrote: --Format Base Type P: Packed -- MESA_FORMAT_[[component list,bit width][storage type*][_]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_R5G6B5_UNORM /* RGGG GGGB */ MESA_FORMAT_B4G4R4X4_UNORM /* */ This is slightly confusing in that the PIPE_FORMATs use this convention for naming the components of 'array' formats, packed formats use BGRX (just like packed MESA_FORMATs do now). Beware that not all PIPE_FORMATs have been updated yet according to that distinction. Is this what you are suggesting then? --Format Base Type P: Packed -- MESA_FORMAT_[[component list][bit width per component]_[storage type*]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_RGB565_UNORM /* RGGG GGGB */ MESA_FORMAT_BGRX_UNORM /* */ MESA_FORMAT_Z32_FLOAT_SX824_UINT MESA_FORMAT_RGBA1010102_UINT MESA_FORMAT_RGBE9995_FLOAT That would be more consistent with the current PIPE_FORMAT naming convention, but some of the component size sequences look a bit weird, your original proposal actually makes more sense for those... Sorry, I'm not sure which original proposal you are referring to, could you be more specific? My concern about this latest iteration is that the delimiter between bit widths per component isn't distinct, though it's generally intuitive, whereas a component followed by a bit width gives a clearer association. I'm afraid there also needs to be a way to encode endianness, either explicitly or via something like _REV to indicate the inverse byte order of the host byte order. This would apply to the packed values as a whole and to any multi-byte components of array formats. I have seen the gallium discussion on this. Since my current focus is i965, I'm safely immune from big endian problems and I'm not aware of the details around the issue. Is it not sufficient to maintain a flag within the individual texture object or image, or a global flag per context to indicate host big endianess? Maybe? Hard to be sure without actually trying it and working out all the issues. Since the endianess issue is still in flux, and the format naming work doesn't offer a silver bullet, my recommendation is to solve the two problems independently. Thus I can produce a new set of patches based on the naming spec tomorrow for another round or review. Mark -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: rename MESA format names to have the same names as PIPE formats
I'd like to propose a draft naming convention for MESA_FORMATs, which, if we can agree on that, then other format systems can be brought to conformance on their own time, after the MESA_FORMATS are fixed. First, there shall be 3 naming format base types: those for component array formats (type A); those for compressed formats (type C); and those for packed component formats (type P). All format names are defined in the mesa_format enum which is found in formats.h. Each format name shall begin with MESA_FORMAT_, followed by a component label (from the Component Label list below) for each component in the order that the component(s) occur in the format, except for non-linear color formats where the first letter shall be 'S'. For type P formats, each component label is followed by the number of bits that represent it in the fundamental data type used by the format. Following the listing of the component labels shall be an underscore; a compression type followed by an underscore for Type C formats only; a storage type from the list below; and a bit with for type A formats, which is the bit width for each array element. --Format Base Type A: Array -- MESA_FORMAT_[component list]_[storage type][array element bit width] examples: MESA_FORMAT_A_SNORM8 /* uchar[i] = A */ MESA_FORMAT_RGBA_UNORM16/* ushort[i * 4 + 0] = R, ushort[i * 4 + 1] = G, ushort[i * 4 + 2] = B, ushort[i * 4 + 3] = A */ MESA_FORMAT_Z_FLOAT32 /* float[i] = Z */ --Format Base Type C: Compressed -- MESA_FORMAT_[component list*][_*][compression type][storage type*] * where required examples: MESA_FORMAT_RGB_ETC1 MESA_FORMAT_RGBA_ETC2 MESA_FORMAT_LATC1_UNORM MESA_FORMAT_RGBA_FXT1 --Format Base Type P: Packed -- MESA_FORMAT_[[component list,bit width][storage type*][_]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_R5G6B5_UNORM /* RGGG GGGB */ MESA_FORMAT_B4G4R4X4_UNORM /* */ MESA_FORMAT_Z32_FLOAT_S8X24_UINT MESA_FORMAT_R10G10B10A2_UINT MESA_FORMAT_R9G9B9E5_FLOAT --Component Labels: -- A - Alpha B - Blue DU - Delta U DV - Delta V E - Shared Exponent G - Green I - Intensity L - Luminescence R - Red S - Stencil (when not followed by RGB or RGBA) U - Chrominance V - Chrominance Y - Luma X - Packing bits Z - Depth --Type C Compression Types: -- DXT1 - Color component labels shall be given DXT3 - Color component labels shall be given DXT5 - Color component labels shall be given ETC1 - No other information required ETC2 - No other information required FXT1 - Color component labels shall be included in name FXT3 - Color component labels shall be included in name LACT1 - fundamental data type shall be given LACT2 - fundamental data type shall be given --Type A and P Storage Types: -- FLOAT SINT UINT SNORM UNORM Does this address all of the concerns? Mark On Wed, Dec 25, 2013 at 11:26 AM, Marek Olšák mar...@gmail.com wrote: The motivation was to use the same names as Gallium and then to fix the Gallium naming. Also, core Mesa is likely to share u_format tools and code generation with Gallium eventually anyway and then all the Mesa format pack and unpack functions will get removed. Given that, I don't see a point in improving Mesa formats alone. Marek On Tue, Dec 24, 2013 at 4:57 AM, Michel Dänzer mic...@daenzer.net wrote: On Son, 2013-12-22 at 03:46 +0100, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com The renaming was driven by the function st_mesa_format_to_pipe_format. Only whole words are renamed to prevent regressions. For the MESA formats which don't have corresponding PIPE formats, I tried to follow the PIPE_FORMAT_* conventions except for a few REV packed formats, whose renaming is left for a future patch. This patch conflicts with Mark's MESA_FORMAT patches, right? Can you guys work out which way you want to take this? :) /* msb -- TEXEL BITS --- lsb */ /* */ [...] + MESA_FORMAT_B8G8R8_UNORM, /* */ + MESA_FORMAT_R8G8B8_UNORM, /* */ [...] + MESA_FORMAT_R8G8B8_SRGB, /* */ I guess these are examples of formats where Mark called out the memory layout documentation comments being wrong. These formats cannot be usefully defined as packed values, so the format names and comments should be changed to reflect that, e.g. per this example: - MESA_FORMAT_SIGNED_RGB_16, /* ushort[0]=R, ushort[1]=G, ushort[2]=B */ - MESA_FORMAT_RGBA_FLOAT32, - MESA_FORMAT_RGBA_FLOAT16, - MESA_FORMAT_RGB_FLOAT32, -
Re: [Mesa-dev] [PATCH] mesa: rename MESA format names to have the same names as PIPE formats
On Wed, Dec 25, 2013 at 7:25 PM, Michel Dänzer mic...@daenzer.net wrote: On Mit, 2013-12-25 at 15:19 -0800, Mark Mueller wrote: --Format Base Type P: Packed -- MESA_FORMAT_[[component list,bit width][storage type*][_]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_R5G6B5_UNORM /* RGGG GGGB */ MESA_FORMAT_B4G4R4X4_UNORM /* */ This is slightly confusing in that the PIPE_FORMATs use this convention for naming the components of 'array' formats, packed formats use BGRX (just like packed MESA_FORMATs do now). Beware that not all PIPE_FORMATs have been updated yet according to that distinction. Is this what you are suggesting then? --Format Base Type P: Packed -- MESA_FORMAT_[[component list][bit width per component]_[storage type*]][_][storage type**] * when type differs between component ** when type applies to all components examples: MESA_FORMAT_RGB565_UNORM /* RGGG GGGB */ MESA_FORMAT_BGRX_UNORM /* */ MESA_FORMAT_Z32_FLOAT_SX824_UINT MESA_FORMAT_RGBA1010102_UINT MESA_FORMAT_RGBE9995_FLOAT I'm afraid there also needs to be a way to encode endianness, either explicitly or via something like _REV to indicate the inverse byte order of the host byte order. This would apply to the packed values as a whole and to any multi-byte components of array formats. I have seen the gallium discussion on this. Since my current focus is i965, I'm safely immune from big endian problems and I'm not aware of the details around the issue. Is it not sufficient to maintain a flag within the individual texture object or image, or a global flag per context to indicate host big endianess? My work with cleaning up the mesa format names is a precursor to adding more formats without associated pack and unpack functions (in other words, beyond MESA_FORMAT_COUNT). If a slew of big endian formats need to be added, they could be included in the same way. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/1] Adjust MESA_FORMAT color component ordering to match API docs
On Fri, Dec 20, 2013 at 7:38 AM, Brian Paul bri...@vmware.com wrote: On 12/19/2013 06:47 PM, Michel Dänzer wrote: On Don, 2013-12-19 at 13:56 -0800, Mark Mueller wrote: Adjust MESA_FORMAT color component ordering to match API docs, driver specific formats (e.g. PIPE_FORMATs), Actually, there are a couple of examples of other format definitions which match the Mesa formats before your change but no longer after it, e.g. in the DRI and i915, nouveau and radeon driver code. Changing the Mesa format definitions will be confusing for people switching between branches with and without your change. Unfortunately, I don't think there's a solution to that issue. There'll always be stable branches that we're cherry-picking to. If that's a major concern, we'll never change any of the MESA_FORMATs. Also, because these Mesa formats are defined as packed values, you're essentially changing the notation from big endian (aka human readable) to little endian. It's unfortunate that the packed PIPE_FORMATs are named in little endian order, that's a concession we had to make when adding them. Overall, I'm afraid this change doesn't look very good at all to me. At the very least though, you'd also have to change the order of component sizes for formats such as MESA_FORMAT_BGRA2101010_UNORM or MESA_FORMAT_BGRA1555_UNORM, otherwise they're just plain wrong. Yeah, I missed that, and I agree that if we're going to rename things to follow the Gallium style, we should fix those too. That wasn't an oversight, that part is in my next phase. If it's preferred, I can move this patch into the subsequent series that I've been working on. I'd prefer not to because the first 2 patches are huge but straight forward changes. The subsequent series is more challenging because further name changing exposes format names that are redundant as well as cases where formats are being used incorrectly, probably because the name was misunderstood. The series will be smaller but will require more thorough review and testing by reviewers. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/1] Adjust MESA_FORMAT color component ordering to match API docs
On Thu, Dec 19, 2013 at 5:47 PM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2013-12-19 at 13:56 -0800, Mark Mueller wrote: Adjust MESA_FORMAT color component ordering to match API docs, driver specific formats (e.g. PIPE_FORMATs), Actually, there are a couple of examples of other format definitions which match the Mesa formats before your change but no longer after it, e.g. in the DRI and i915, nouveau and radeon driver code. It's just not fair to hold the mesa formats to the varied conventions in the DRI driver code because they are all over the place across all the different drivers, past, present, and future (the confusion with MESA_FORMATs may be partly to blame). Moving the mesa formats closer to those of the API, instead of a nasty mix in-between, brings at least some clarity to the situation and the documentation burden can be left to the API specs - minus a few outliers. I'm pretty confident that this naming confusion is resulting in some unnecessary texture re-packing work due to _mesa_choose_tex_format returning less then optimal formats, though by name they look to be the best choice. My next series attempts to address that. Changing the Mesa format definitions will be confusing for people switching between branches with and without your change. Also, because these Mesa formats are defined as packed values, you're essentially changing the notation from big endian (aka human readable) to little endian. It's unfortunate that the packed PIPE_FORMATs are named in little endian order, that's a concession we had to make when adding them. Are they all really big endian, currently it looks like a mix of who knows what. The main role the MESA_FORMATs are serving is to fortify the GLenums coming from the API, which are close to useless on their own. I don't think there is any question that there is a lack of convention and using the conventions already provided by the API offers the best fit for lots of reasons. Overall, I'm afraid this change doesn't look very good at all to me. At the very least though, you'd also have to change the order of component sizes for formats such as MESA_FORMAT_BGRA2101010_UNORM or MESA_FORMAT_BGRA1555_UNORM, otherwise they're just plain wrong. and actual use on common platforms. What does that mean? I'm referring to PIPE_FORMAT, BRW_SURFACEFORMAT, RADEON_TXFORMAT, etc here. Remove comments giving MESA_FORMAT color packings, some of which are misleading. Which ones are misleading, and how? Bottom line, if the MESA_FORMATs follow the API's convention, then the reader can go to the API for clarification and we don't have to maintain that in the code. There are some special cases which can be dealt with in the code. diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h index 94fc7a0..f224ed5 100644 --- a/src/mesa/main/formats.h +++ b/src/mesa/main/formats.h @@ -62,67 +62,68 @@ typedef enum MESA_FORMAT_NONE = 0, /** -* \name Basic hardware formats +* \name Basic API user space data formats All of Mesa is in user space. :) +* Please refer to API documentation for more information on format +* packing */ What API documentation? opengl.org/wiki/Pixel_Transfer opengl.org/wiki/Image_Format http://msdn.microsoft.com/en-us/library/windows/desktop/bb172558(v=vs.85).aspx Thanks for your input Michel. Do you still think this is a bad idea? Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [V2 PATCH 1/2] mesa: inline r200 radeon texture format macros to facility search and replace
Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/r200/r200_texstate.c | 108 +++--- src/mesa/drivers/dri/radeon/radeon_texstate.c | 64 ++- 2 files changed, 70 insertions(+), 102 deletions(-) diff --git a/src/mesa/drivers/dri/r200/r200_texstate.c b/src/mesa/drivers/dri/r200/r200_texstate.c index b20bd51..b89bb39 100644 --- a/src/mesa/drivers/dri/r200/r200_texstate.c +++ b/src/mesa/drivers/dri/r200/r200_texstate.c @@ -60,20 +60,8 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #define R200_TXFORMAT_RGBA_DXT3 R200_TXFORMAT_DXT23 #define R200_TXFORMAT_RGBA_DXT5 R200_TXFORMAT_DXT45 -#define _COLOR(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f, 0 } -#define _COLOR_REV(f) \ -[ MESA_FORMAT_ ## f ## _REV ] = { R200_TXFORMAT_ ## f, 0 } -#define _ALPHA(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f | R200_TXFORMAT_ALPHA_IN_MAP, 0 } -#define _ALPHA_REV(f) \ -[ MESA_FORMAT_ ## f ## _REV ] = { R200_TXFORMAT_ ## f | R200_TXFORMAT_ALPHA_IN_MAP, 0 } -#define _YUV(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f, R200_YUV_TO_RGB } -#define _INVALID(f) \ -[ MESA_FORMAT_ ## f ] = { 0x, 0 } #define VALID_FORMAT(f) ( ((f) = MESA_FORMAT_RGBA_DXT5) \ - (tx_table_be[f].format != 0x) ) + (tx_table_be[f].format != 0x) ) struct tx_table { GLuint format, filter; @@ -82,63 +70,59 @@ struct tx_table { static const struct tx_table tx_table_be[] = { [ MESA_FORMAT_RGBA ] = { R200_TXFORMAT_ABGR | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, - _ALPHA_REV(RGBA), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _INVALID(RGB888), - _COLOR(RGB565), - _COLOR_REV(RGB565), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _ALPHA(ARGB1555), - _ALPHA_REV(ARGB1555), - _ALPHA(AL88), - _ALPHA_REV(AL88), - _ALPHA(A8), - _COLOR(L8), - _ALPHA(I8), - _YUV(YCBCR), - _YUV(YCBCR_REV), - _INVALID(RGB_FXT1), - _INVALID(RGBA_FXT1), - _COLOR(RGB_DXT1), - _ALPHA(RGBA_DXT1), - _ALPHA(RGBA_DXT3), - _ALPHA(RGBA_DXT5), + [ MESA_FORMAT_RGBA_REV ] = { R200_TXFORMAT_RGBA | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGB888 ] = { 0x, 0 }, + [ MESA_FORMAT_RGB565 ] = { R200_TXFORMAT_RGB565, 0 }, + [ MESA_FORMAT_RGB565_REV ] = { R200_TXFORMAT_RGB565, 0 }, + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB1555 ] = { R200_TXFORMAT_ARGB1555 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB1555_REV ] = { R200_TXFORMAT_ARGB1555 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_AL88 ] = { R200_TXFORMAT_AL88 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_AL88_REV ] = { R200_TXFORMAT_AL88 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_A8 ] = { R200_TXFORMAT_A8 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_L8 ] = { R200_TXFORMAT_L8, 0 }, + [ MESA_FORMAT_I8 ] = { R200_TXFORMAT_I8 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_YCBCR ] = { R200_TXFORMAT_YCBCR, R200_YUV_TO_RGB }, + [ MESA_FORMAT_YCBCR_REV ] = { R200_TXFORMAT_YCBCR_REV, R200_YUV_TO_RGB }, + [ MESA_FORMAT_RGB_FXT1 ] = { 0x, 0 }, + [ MESA_FORMAT_RGBA_FXT1 ] = { 0x, 0 }, + [ MESA_FORMAT_RGB_DXT1 ] = { R200_TXFORMAT_RGB_DXT1, 0 }, + [ MESA_FORMAT_RGBA_DXT1 ] = { R200_TXFORMAT_RGBA_DXT1 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGBA_DXT3 ] = { R200_TXFORMAT_RGBA_DXT3 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGBA_DXT5 ] = { R200_TXFORMAT_RGBA_DXT5 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, }; static const struct tx_table tx_table_le[] = { - _ALPHA(RGBA), + [ MESA_FORMAT_RGBA ] = { R200_TXFORMAT_RGBA | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, [ MESA_FORMAT_RGBA_REV ] = { R200_TXFORMAT_ABGR | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, - _ALPHA(ARGB), - _ALPHA_REV(ARGB), + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, [ MESA_FORMAT_RGB888 ] = { R200_TXFORMAT_ARGB, 0 }, - _COLOR(RGB565), - _COLOR_REV(RGB565), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _ALPHA(ARGB1555), - _ALPHA_REV(ARGB1555), - _ALPHA(AL88), - _ALPHA_REV(AL88), - _ALPHA(A8), - _COLOR(L8), - _ALPHA(I8), - _YUV(YCBCR), - _YUV(YCBCR_REV), - _INVALID(RGB_FXT1), - _INVALID(RGBA_FXT1), - _COLOR(RGB_DXT1), - _ALPHA(RGBA_DXT1), - _ALPHA(RGBA_DXT3), - _ALPHA(RGBA_DXT5), + [ MESA_FORMAT_RGB565 ] = { R200_TXFORMAT_RGB565, 0
[Mesa-dev] [V2 PATCH 1/2] mesa: inline r200 radeon texture format macros to facility search and replace
v2: This patch was inserted into the series to correct build problems with r200 and radeon drivers. The patch replaces macros that operate on the MESA_FORMATs with their inline equivalent to facilitate search and replace. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/r200/r200_texstate.c | 108 +++--- src/mesa/drivers/dri/radeon/radeon_texstate.c | 64 ++- 2 files changed, 70 insertions(+), 102 deletions(-) diff --git a/src/mesa/drivers/dri/r200/r200_texstate.c b/src/mesa/drivers/dri/r200/r200_texstate.c index b20bd51..b89bb39 100644 --- a/src/mesa/drivers/dri/r200/r200_texstate.c +++ b/src/mesa/drivers/dri/r200/r200_texstate.c @@ -60,20 +60,8 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #define R200_TXFORMAT_RGBA_DXT3 R200_TXFORMAT_DXT23 #define R200_TXFORMAT_RGBA_DXT5 R200_TXFORMAT_DXT45 -#define _COLOR(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f, 0 } -#define _COLOR_REV(f) \ -[ MESA_FORMAT_ ## f ## _REV ] = { R200_TXFORMAT_ ## f, 0 } -#define _ALPHA(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f | R200_TXFORMAT_ALPHA_IN_MAP, 0 } -#define _ALPHA_REV(f) \ -[ MESA_FORMAT_ ## f ## _REV ] = { R200_TXFORMAT_ ## f | R200_TXFORMAT_ALPHA_IN_MAP, 0 } -#define _YUV(f) \ -[ MESA_FORMAT_ ## f ] = { R200_TXFORMAT_ ## f, R200_YUV_TO_RGB } -#define _INVALID(f) \ -[ MESA_FORMAT_ ## f ] = { 0x, 0 } #define VALID_FORMAT(f) ( ((f) = MESA_FORMAT_RGBA_DXT5) \ - (tx_table_be[f].format != 0x) ) + (tx_table_be[f].format != 0x) ) struct tx_table { GLuint format, filter; @@ -82,63 +70,59 @@ struct tx_table { static const struct tx_table tx_table_be[] = { [ MESA_FORMAT_RGBA ] = { R200_TXFORMAT_ABGR | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, - _ALPHA_REV(RGBA), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _INVALID(RGB888), - _COLOR(RGB565), - _COLOR_REV(RGB565), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _ALPHA(ARGB1555), - _ALPHA_REV(ARGB1555), - _ALPHA(AL88), - _ALPHA_REV(AL88), - _ALPHA(A8), - _COLOR(L8), - _ALPHA(I8), - _YUV(YCBCR), - _YUV(YCBCR_REV), - _INVALID(RGB_FXT1), - _INVALID(RGBA_FXT1), - _COLOR(RGB_DXT1), - _ALPHA(RGBA_DXT1), - _ALPHA(RGBA_DXT3), - _ALPHA(RGBA_DXT5), + [ MESA_FORMAT_RGBA_REV ] = { R200_TXFORMAT_RGBA | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGB888 ] = { 0x, 0 }, + [ MESA_FORMAT_RGB565 ] = { R200_TXFORMAT_RGB565, 0 }, + [ MESA_FORMAT_RGB565_REV ] = { R200_TXFORMAT_RGB565, 0 }, + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB1555 ] = { R200_TXFORMAT_ARGB1555 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB1555_REV ] = { R200_TXFORMAT_ARGB1555 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_AL88 ] = { R200_TXFORMAT_AL88 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_AL88_REV ] = { R200_TXFORMAT_AL88 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_A8 ] = { R200_TXFORMAT_A8 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_L8 ] = { R200_TXFORMAT_L8, 0 }, + [ MESA_FORMAT_I8 ] = { R200_TXFORMAT_I8 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_YCBCR ] = { R200_TXFORMAT_YCBCR, R200_YUV_TO_RGB }, + [ MESA_FORMAT_YCBCR_REV ] = { R200_TXFORMAT_YCBCR_REV, R200_YUV_TO_RGB }, + [ MESA_FORMAT_RGB_FXT1 ] = { 0x, 0 }, + [ MESA_FORMAT_RGBA_FXT1 ] = { 0x, 0 }, + [ MESA_FORMAT_RGB_DXT1 ] = { R200_TXFORMAT_RGB_DXT1, 0 }, + [ MESA_FORMAT_RGBA_DXT1 ] = { R200_TXFORMAT_RGBA_DXT1 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGBA_DXT3 ] = { R200_TXFORMAT_RGBA_DXT3 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_RGBA_DXT5 ] = { R200_TXFORMAT_RGBA_DXT5 | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, }; static const struct tx_table tx_table_le[] = { - _ALPHA(RGBA), + [ MESA_FORMAT_RGBA ] = { R200_TXFORMAT_RGBA | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, [ MESA_FORMAT_RGBA_REV ] = { R200_TXFORMAT_ABGR | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, - _ALPHA(ARGB), - _ALPHA_REV(ARGB), + [ MESA_FORMAT_ARGB ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, + [ MESA_FORMAT_ARGB_REV ] = { R200_TXFORMAT_ARGB | R200_TXFORMAT_ALPHA_IN_MAP, 0 }, [ MESA_FORMAT_RGB888 ] = { R200_TXFORMAT_ARGB, 0 }, - _COLOR(RGB565), - _COLOR_REV(RGB565), - _ALPHA(ARGB), - _ALPHA_REV(ARGB), - _ALPHA(ARGB1555), - _ALPHA_REV(ARGB1555), - _ALPHA(AL88), - _ALPHA_REV(AL88), - _ALPHA(A8), - _COLOR(L8), - _ALPHA(I8), - _YUV(YCBCR), - _YUV
Re: [Mesa-dev] [PATCH 1/1] Adjust MESA_FORMAT color component ordering to match API docs
No piglit regressions on i965, or autotools based build regressions on ilo,nouveau,r300,r600 were observed as a result of these changes. Mark On Thu, Dec 19, 2013 at 1:56 PM, Mark Mueller markkmuel...@gmail.comwrote: Adjust MESA_FORMAT color component ordering to match API docs, driver specific formats (e.g. PIPE_FORMATs), and actual use on common platforms. Add comment to reference API docs. Remove comments giving MESA_FORMAT color packings, some of which are misleading. The MESA_FORMAT changes are as follows: s/MESA_FORMAT_ARGB/MESA_FORMAT_BGRA/g s/MESA_FORMAT_ABGR/MESA_FORMAT_RGBA/g s/MESA_FORMAT_XRGB/MESA_FORMAT_BGRX/G MESA_FORMAT_XBGR was purposefully emitted because it exposes the redundant MESA_FORMAT_XBGR_SNORM and thus requires a more extensive patch which will be submitted separately. This patch apply on top of the previous MESA_FORMAT change. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/gallium/state_trackers/dri/common/dri_screen.c | 4 +- src/mesa/drivers/dri/common/dri_util.c | 16 +- src/mesa/drivers/dri/common/utils.c| 16 +- src/mesa/drivers/dri/i915/i830_texstate.c | 8 +- src/mesa/drivers/dri/i915/i830_vtbl.c | 8 +- src/mesa/drivers/dri/i915/i915_context.c | 8 +- src/mesa/drivers/dri/i915/i915_texstate.c | 8 +- src/mesa/drivers/dri/i915/i915_vtbl.c | 8 +- src/mesa/drivers/dri/i915/intel_blit.c | 20 +-- src/mesa/drivers/dri/i915/intel_pixel_bitmap.c | 4 +- src/mesa/drivers/dri/i915/intel_screen.c | 10 +- src/mesa/drivers/dri/i915/intel_tex_image.c| 4 +- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 8 +- src/mesa/drivers/dri/i965/brw_context.c| 2 +- src/mesa/drivers/dri/i965/brw_surface_formats.c| 28 ++-- src/mesa/drivers/dri/i965/intel_blit.c | 12 +- src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 4 +- src/mesa/drivers/dri/i965/intel_screen.c | 8 +- src/mesa/drivers/dri/i965/intel_tex_image.c| 4 +- src/mesa/drivers/dri/i965/intel_tex_subimage.c | 2 +- src/mesa/drivers/dri/nouveau/nouveau_fbo.c | 4 +- src/mesa/drivers/dri/nouveau/nouveau_screen.c | 4 +- src/mesa/drivers/dri/nouveau/nouveau_texture.c | 12 +- src/mesa/drivers/dri/nouveau/nouveau_util.h| 8 +- src/mesa/drivers/dri/nouveau/nv04_context.c| 2 +- src/mesa/drivers/dri/nouveau/nv04_state_fb.c | 4 +- src/mesa/drivers/dri/nouveau/nv04_state_frag.c | 2 +- src/mesa/drivers/dri/nouveau/nv04_state_raster.c | 2 +- src/mesa/drivers/dri/nouveau/nv04_state_tex.c | 8 +- src/mesa/drivers/dri/nouveau/nv04_surface.c| 56 +++ src/mesa/drivers/dri/nouveau/nv10_state_fb.c | 4 +- src/mesa/drivers/dri/nouveau/nv10_state_frag.c | 4 +- src/mesa/drivers/dri/nouveau/nv10_state_tex.c | 14 +- src/mesa/drivers/dri/nouveau/nv20_state_fb.c | 4 +- src/mesa/drivers/dri/nouveau/nv20_state_tex.c | 16 +- src/mesa/drivers/dri/r200/r200_blit.c | 32 ++-- src/mesa/drivers/dri/r200/r200_state_init.c| 4 +- src/mesa/drivers/dri/r200/r200_texstate.c | 6 +- src/mesa/drivers/dri/radeon/radeon_blit.c | 24 +-- src/mesa/drivers/dri/radeon/radeon_pixel_read.c| 12 +- src/mesa/drivers/dri/radeon/radeon_screen.c| 16 +- src/mesa/drivers/dri/radeon/radeon_state_init.c| 4 +- src/mesa/drivers/dri/radeon/radeon_tex_copy.c | 4 +- src/mesa/drivers/dri/radeon/radeon_texstate.c | 6 +- src/mesa/drivers/dri/radeon/radeon_texture.c | 12 +- src/mesa/drivers/dri/swrast/swrast.c | 14 +- src/mesa/drivers/haiku/swrast/SoftwareRast.cpp | 6 +- src/mesa/drivers/osmesa/osmesa.c | 8 +- src/mesa/drivers/x11/xm_buffer.c | 6 +- src/mesa/main/debug.c | 2 +- src/mesa/main/fbobject.c | 2 +- src/mesa/main/format_pack.c| 94 +-- src/mesa/main/format_unpack.c | 54 +++--- src/mesa/main/formats.c| 114 ++--- src/mesa/main/formats.h| 183 ++--- src/mesa/main/framebuffer.c| 2 +- src/mesa/main/readpix.c| 2 +- src/mesa/main/texcompress_etc.c| 2 +- src/mesa/main/texformat.c | 50 +++--- src/mesa/main/texstore.c | 72 src/mesa/state_tracker/st_format.c | 44 ++--- src/mesa/swrast/s_drawpix.c| 12 +- src/mesa/swrast/s_texfetch.c | 28 ++-- src/mesa/swrast/s_texfetch_tmp.h | 18 +- 64 files
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Wed, Dec 18, 2013 at 7:36 AM, Brian Paul bri...@vmware.com wrote: On 12/17/2013 07:50 PM, Mark Mueller wrote: On Tue, Dec 17, 2013 at 12:19 PM, Mark Mueller markkmuel...@gmail.com mailto:markkmuel...@gmail.com wrote: On Sat, Nov 23, 2013 at 4:10 PM, Marek Olšák mar...@gmail.com mailto:mar...@gmail.com wrote: On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller markkmuel...@gmail.com mailto:markkmuel...@gmail.com wrote: On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák mar...@gmail.com mailto:mar...@gmail.com wrote: [..] OK, I think I've realized why this is so difficult. There are some MESA_FORMAT component orders that are counter to their OGL counterparts in name, and the same appears true for the bit count numberings. Just FYI: there's no intention that MESA_FORMATs match any OpenGL format/type/internalFormat. MESA_FORMATs are intended to match what the hardware wants. Ideally, we hit TexImage/etc paths where the user-specified format/type/internalFormat exactly matches a MESA_FORMAT to avoid conversion/swizzling. Back in the early days of OpenGL, most OpenGL formats directly corresponded to SGI hardware formats. Over time, as PC GPUs arrived, newer formats (like GL_BGR[A]) were added. Throw in big vs. little endian issues and it's kind of a mess. For example these two cases in _mesa_choose_tex_format: case GL_BGRA: RETURN_IF_SUPPORTED(MESA_FORMAT_ARGB); vs. case GL_RGBA32F_ARB: RETURN_IF_SUPPORTED(MESA_FORMAT_RGBA_FLOAT32); Part of the issue here is do you treat the pixel/texel as a packed value or as an array of values? Most of the 4-byte rgba formats expect texels to be treated as packed 4-byte words while the 16-byte floating point format is treated as an array of four floats. That leads to confusion too. and Mesa defines these: MESA_FORMAT_ARGB1555,/* ARRR RRGG GGGB */ MESA_FORMAT_ARGB1555_REV,/* GGGB ARRR RRGG */ while in OGL it's this way: GL_UNSIGNED_SHORT_5_5_5_1 GL_UNSIGNED_SHORT_1_5_5_5_REV Again, the apparent inconsistency comes from old OpenGL (SGI) conventions vs. PC hardware conventions. I'll modify my additions to better match Mesa's convention and hopefully that will clear a few things up. Or would it be better to fix this dilemma once and for all? I've heard Ken suggesting that that be done. It has been causing me so much grief that I'd _love_ to eliminate the problem but would rather move on if I can't get buy in. I guess I'm still not clear on what the new MESA_FORMATs are supposed to represent? API/user-space data or hardware/GPU data? Or both? -Brian Yes, the confusion is definitely deeper than the naming convention, which is all the more disorienting, but I can see many sound reasons for MESA_FORMATs to directly follow the API/user-space naming conventions: - A vast majority of MESA_FORMATs already match their API/user-space compatriots, their primary role is to represent user-space data formats, and that is where their meta-data is most useful. - The PIPE_FORMATs and BRW_SURFACEFORMATS serve better for hardware/GPU specificity. - The API formats are already well defined and documented, trying to reach a similar nirvana among the various hardware formats within MESA_FORMATs would be hard work. - Hardware formats are opaque within core Mesa and they are vastly complex for orthogonal reasons (like formats that can be sampled from but not rendered to, along with 7 other parameters), so this opacity is a good thing. i965 uses BRW_SURFACEFORMATS which efficiently map to MESA_FORMATs and the _mesa_choose_tex_format methodology does a passable job at making it all work - Modern hardware can efficiently handle most, if not all, formats thrown at it so today's limits are now completely defined and maintained by the API. Color component ordering is becoming irrelevant short of knowing the order it is received from user-space. So is that convincing enough to justify a more direct mapping of MESA_FORMAT names to API names? Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
Towards this end, I'd like to start by adding UNORM, or UINT, where applicable, to all of the MESA_FORMATs listed in the basic hardware formats section. Are there any objections? Mark On Wed, Dec 18, 2013 at 2:16 AM, Marek Olšák mar...@gmail.com wrote: See Gallium PIPE_FORMATs. They all have consistent names. The translation between Mesa and Gallium formats takes place in st_format.c. I'm inclined to rename all Mesa formats to match the Gallium names. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Sat, Nov 23, 2013 at 4:10 PM, Marek Olšák mar...@gmail.com wrote: On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller markkmuel...@gmail.com wrote: On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák mar...@gmail.com wrote: [...] MESA_FORMAT_RGBA1555_REV, I don't think you can have R with 1 bit. /* Blue Green Red Alpha */ MESA_FORMAT_BGRA_INT8, MESA_FORMAT_BGRA_INT8_REV, MESA_FORMAT_BGRA_UINT8, MESA_FORMAT_BGRA_INT16, MESA_FORMAT_BGRA_UINT16, MESA_FORMAT_BGRA_FLOAT16, MESA_FORMAT_BGRA_INT32, MESA_FORMAT_BGRA_UINT32, MESA_FORMAT_BGRA_FLOAT32, MESA_FORMAT_BGRA, MESA_FORMAT_BGRA_REV, MESA_FORMAT_BGRA5551, All these look good. MESA_FORMAT_BGRA1555_REV, I don't think you can have B with 1 bit. Doesn't the _REV force A to be the first component, i.e. 1 bit? This format is listed as valid in http://www.opengl.org/wiki/Pixel_Transfer - see below. MESA_FORMAT_BGRA1010102, This one looks good. MESA_FORMAT_BGRA2101010_REV, I don't think you can have B with 2 bits. Same as above. MESA_FORMAT_BGRA5999_REV, This one doesn't exist either and cannot be specified by TexImage. You are correct according to the Pixel_Transfer spec, thanks. BGRAs and RGBAs were based on this from the spec: GL_INVALID_OPERATION is generated if type is one of GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_SHORT_4_4_4_4_REV, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_SHORT_1_5_5_5_REV, GL_UNSIGNED_INT_8_8_8_8, GL_UNSIGNED_INT_8_8_8_8_REV, GL_UNSIGNED_INT_10_10_10_2, GL_UNSIGNED_INT_2_10_10_10_REV, or GL_UNSIGNED_INT_5_9_9_9_REV, and format is neither GL_RGBA nor GL_BGRA. Nowhere can I see that B has 1 or 2 bits. Also the occurence of GL_UNSIGNED_INT_5_9_9_9_REV seems to be a spec bug, because it should be GL_RGB (the 5 bits contain a shared exponent). It does appear to be a spec bug. Here is a quote from http://www.opengl.org/wiki/Pixel_Transfer which addresses that an other issues above: OpenGL restricts the possible sizes. They are: * 3_3_2 (2_3_3_REV): unsigned bytes. Only used with GL_RGB. * 5_6_5 (5_6_5_REV): unsigned shorts. Only used with GL_RGB. * 4_4_4_4 (4_4_4_4_REV): unsigned shorts. * 5_5_5_1 (1_5_5_5_REV): unsigned shorts. * 8_8_8_8 (8_8_8_8_REV): unsigned ints. * 10_10_10_2 (2_10_10_10_REV): unsigned ints. * 24_8 (no _REV): unsigned ints. Only used with GL_DEPTH_STENCIL. * 10F_11F_11F_REV (no non-REV): unsigned ints. These represent floats, and can only be used with GL_RGB. This should only be used with images that have the GL_R11F_G11F_B10F image format. * 5_9_9_9_REV (no non-REV): unsigned ints. Only used with GL_RGB; the last component (the 5. It's REV) does not directly map to a color value. It is a shared exponent. Only use this with images that have the GL_RGB9_E5 image format. I'm working on V2 of this patch with incorporation of Marek, Brian's, and Roland's feedback. I have also made changes as a result of further development and testing on the i965 GPU texture upload work. I will be posting that shortly. Thanks Marek. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Tue, Dec 17, 2013 at 12:19 PM, Mark Mueller markkmuel...@gmail.comwrote: On Sat, Nov 23, 2013 at 4:10 PM, Marek Olšák mar...@gmail.com wrote: On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller markkmuel...@gmail.com wrote: On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák mar...@gmail.com wrote: [...] MESA_FORMAT_RGBA1555_REV, I don't think you can have R with 1 bit. /* Blue Green Red Alpha */ MESA_FORMAT_BGRA_INT8, MESA_FORMAT_BGRA_INT8_REV, MESA_FORMAT_BGRA_UINT8, MESA_FORMAT_BGRA_INT16, MESA_FORMAT_BGRA_UINT16, MESA_FORMAT_BGRA_FLOAT16, MESA_FORMAT_BGRA_INT32, MESA_FORMAT_BGRA_UINT32, MESA_FORMAT_BGRA_FLOAT32, MESA_FORMAT_BGRA, MESA_FORMAT_BGRA_REV, MESA_FORMAT_BGRA5551, All these look good. MESA_FORMAT_BGRA1555_REV, I don't think you can have B with 1 bit. Doesn't the _REV force A to be the first component, i.e. 1 bit? This format is listed as valid in http://www.opengl.org/wiki/Pixel_Transfer - see below. MESA_FORMAT_BGRA1010102, This one looks good. MESA_FORMAT_BGRA2101010_REV, I don't think you can have B with 2 bits. Same as above. MESA_FORMAT_BGRA5999_REV, This one doesn't exist either and cannot be specified by TexImage. You are correct according to the Pixel_Transfer spec, thanks. BGRAs and RGBAs were based on this from the spec: GL_INVALID_OPERATION is generated if type is one of GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_SHORT_4_4_4_4_REV, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_SHORT_1_5_5_5_REV, GL_UNSIGNED_INT_8_8_8_8, GL_UNSIGNED_INT_8_8_8_8_REV, GL_UNSIGNED_INT_10_10_10_2, GL_UNSIGNED_INT_2_10_10_10_REV, or GL_UNSIGNED_INT_5_9_9_9_REV, and format is neither GL_RGBA nor GL_BGRA. Nowhere can I see that B has 1 or 2 bits. Also the occurence of GL_UNSIGNED_INT_5_9_9_9_REV seems to be a spec bug, because it should be GL_RGB (the 5 bits contain a shared exponent). It does appear to be a spec bug. Here is a quote from http://www.opengl.org/wiki/Pixel_Transfer which addresses that an other issues above: OpenGL restricts the possible sizes. They are: * 3_3_2 (2_3_3_REV): unsigned bytes. Only used with GL_RGB. * 5_6_5 (5_6_5_REV): unsigned shorts. Only used with GL_RGB. * 4_4_4_4 (4_4_4_4_REV): unsigned shorts. * 5_5_5_1 (1_5_5_5_REV): unsigned shorts. * 8_8_8_8 (8_8_8_8_REV): unsigned ints. * 10_10_10_2 (2_10_10_10_REV): unsigned ints. * 24_8 (no _REV): unsigned ints. Only used with GL_DEPTH_STENCIL. * 10F_11F_11F_REV (no non-REV): unsigned ints. These represent floats, and can only be used with GL_RGB. This should only be used with images that have the GL_R11F_G11F_B10F image format. * 5_9_9_9_REV (no non-REV): unsigned ints. Only used with GL_RGB; the last component (the 5. It's REV) does not directly map to a color value. It is a shared exponent. Only use this with images that have the GL_RGB9_E5 image format. I'm working on V2 of this patch with incorporation of Marek, Brian's, and Roland's feedback. I have also made changes as a result of further development and testing on the i965 GPU texture upload work. I will be posting that shortly. Thanks Marek. OK, I think I've realized why this is so difficult. There are some MESA_FORMAT component orders that are counter to their OGL counterparts in name, and the same appears true for the bit count numberings. For example these two cases in _mesa_choose_tex_format: case GL_BGRA: RETURN_IF_SUPPORTED(MESA_FORMAT_ARGB); vs. case GL_RGBA32F_ARB: RETURN_IF_SUPPORTED(MESA_FORMAT_RGBA_FLOAT32); and Mesa defines these: MESA_FORMAT_ARGB1555, /* ARRR RRGG GGGB */ MESA_FORMAT_ARGB1555_REV, /* GGGB ARRR RRGG */ while in OGL it's this way: GL_UNSIGNED_SHORT_5_5_5_1 GL_UNSIGNED_SHORT_1_5_5_5_REV I'll modify my additions to better match Mesa's convention and hopefully that will clear a few things up. Or would it be better to fix this dilemma once and for all? I've heard Ken suggesting that that be done. It has been causing me so much grief that I'd _love_ to eliminate the problem but would rather move on if I can't get buy in. Cheers, Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/11] mesa: Add MESA_FORMAT_ABGR2101010.
On Mon, Dec 9, 2013 at 3:09 PM, Paul Berry stereotype...@gmail.com wrote: On 7 December 2013 07:42, Francisco Jerez curroje...@riseup.net wrote: Paul Berry stereotype...@gmail.com writes: On 24 November 2013 21:00, Francisco Jerez curroje...@riseup.net wrote: Including pack/unpack and texstore code. This texture format is a requirement for ARB_shader_image_load_store. --- src/mesa/main/format_pack.c | 29 +++ src/mesa/main/format_unpack.c | 32 ++ src/mesa/main/formats.c | 19 ++ src/mesa/main/formats.h | 2 ++ src/mesa/main/texstore.c | 46 +++ src/mesa/swrast/s_texfetch.c | 6 ++ 6 files changed, 134 insertions(+) diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c index 826fc10..9b6929d 100644 --- a/src/mesa/main/format_pack.c +++ b/src/mesa/main/format_pack.c @@ -1824,6 +1824,31 @@ pack_float_XBGR32323232_FLOAT(const GLfloat src[4], void *dst) d[3] = 1.0; } +/* MESA_FORMAT_ABGR2101010 */ + +static void +pack_ubyte_ABGR2101010(const GLubyte src[4], void *dst) +{ + GLuint *d = ((GLuint *) dst); + GLushort r = UBYTE_TO_USHORT(src[RCOMP]); + GLushort g = UBYTE_TO_USHORT(src[GCOMP]); + GLushort b = UBYTE_TO_USHORT(src[BCOMP]); + GLushort a = UBYTE_TO_USHORT(src[ACOMP]); + *d = PACK_COLOR_2101010_US(a, b, g, r); I don't know if we care, but this conversion is not as accurate as it could be. For example, if the input has an r value of 0x3f, then a perfect conversion would convert this to a float by dividing by 255.0 (to get 0.24706), then convert to 10 bits by multiplying by 1023.0 (to get 252.74), and then round to the nearest integer, which is 253, or 0xfd. However, what the function above does is convert to 16 bits (0x3f3f), then chop off the lower 6 bits by downshifting, to get 0xfc, or 252. I don't think this has to do with using ushorts rather than floats, both have more than enough precision to calculate the result exactly. I believe that this is a general problem that affects many of the users of the PACK_COLOR_* macros because of their use of truncation rather than rounding. If we care, we should probably fix it there. Yeah, that's a good point. And obviously there's no need to address that in this patch series :) [...] @@ -3362,6 +3376,11 @@ _mesa_format_matches_format_and_type(gl_format gl_format, case MESA_FORMAT_XBGR32323232_UINT: case MESA_FORMAT_XBGR32323232_SINT: return GL_FALSE; + + case MESA_FORMAT_ABGR2101010: + return format == GL_RGBA type == GL_UNSIGNED_INT_2_10_10_10_REV + !swapBytes; + I don't understand the byte ordering conventions in this code well enough to confirm that this is correct. According to the GL spec, the UNSIGNED_INT_2_10_10_10_REV type has the first component (R for the GL_RGBA format) starting from bit 0, the second component (G) from bit 10, the third component (B) from bit 20, and the third component (A) from bit 30 of a 32-bit unsigned int, which matches the Mesa format exactly. Ok, I learned a bit more about the conventions used in the MESA_FORMAT_* enum (inconsistent though they are) and double checked your logic; it looks good to me. Consider the patch: Reviewed-by: Paul Berry stereotype...@gmail.com Do you have a unit test that validates that the format is correctly interpreted? Nope, sorry. glean --tests pixelFormats includes a test with this format. [...] ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák mar...@gmail.com wrote: If there is actually hardware support, it's okay to add those formats. See comments below. On Sat, Nov 23, 2013 at 3:59 AM, Mark Mueller markkmuel...@gmail.com wrote: How about let's forget the whole concept of GPU loading for a moment as that is only clouding the issue. As you said: the mesa_* formats have one purpose: to be used as OpenGL textures When I read the OGL spec for glTexImage2D it lists GL_BGR as a format. Yet looking at mesa_* formats there is only one BGR format represented: MESA_FORMAT_BGR888. There are 8 other valid OGL BGR textures that don't have MESA_FORMATs while all of the RGBs ones _are_ all there? ie. these are the OGL BGR formats that are missing: MESA_FORMAT_BGR_INT8, MESA_FORMAT_BGR_UINT8, MESA_FORMAT_BGR_INT16, MESA_FORMAT_BGR_UINT16, MESA_FORMAT_BGR_FLOAT16, MESA_FORMAT_BGR_INT32, MESA_FORMAT_BGR_UINT32, MESA_FORMAT_BGR_FLOAT32 Our hardware doesn't support these formats, because they are not aligned to a power-of-two number of components. If your hardware can do them, it's okay to add them. There are also RGB, RGBA, and BGRA formats missing. Completeness seems like justification enough for their inclusion: /* Red Green Blue */ MESA_FORMAT_RGB233_REV, Same as above. Also please don't use _REV, use BGR332 instead. Definitely better. MESA_FORMAT_RGB10_REV, This format doesn't exist. I don't think you can specify it in glTexImage. The proposed new RGB ones are based on this statement in the spec: GL_INVALID_OPERATION is generated if type is one of GL_UNSIGNED_BYTE_3_3_2, GL_UNSIGNED_BYTE_2_3_3_REV, GL_UNSIGNED_SHORT_5_6_5, GL_UNSIGNED_SHORT_5_6_5_REV, or GL_UNSIGNED_INT_10F_11F_11F_REV, and format is not GL_RGB. /* Red Green Blue Alpha */ MESA_FORMAT_RGBA1010102, This one might be useful. MESA_FORMAT_RGBA2101010_REV, I don't think you can have R with 2 bits. MESA_FORMAT_RGBA5999_REV, This format doesn't exist either. MESA_FORMAT_RGBA, MESA_FORMAT_RGBA_REV, These might be useful. MESA_FORMAT_RGBA1555_REV, I don't think you can have R with 1 bit. /* Blue Green Red Alpha */ MESA_FORMAT_BGRA_INT8, MESA_FORMAT_BGRA_INT8_REV, MESA_FORMAT_BGRA_UINT8, MESA_FORMAT_BGRA_INT16, MESA_FORMAT_BGRA_UINT16, MESA_FORMAT_BGRA_FLOAT16, MESA_FORMAT_BGRA_INT32, MESA_FORMAT_BGRA_UINT32, MESA_FORMAT_BGRA_FLOAT32, MESA_FORMAT_BGRA, MESA_FORMAT_BGRA_REV, MESA_FORMAT_BGRA5551, All these look good. MESA_FORMAT_BGRA1555_REV, I don't think you can have B with 1 bit. MESA_FORMAT_BGRA1010102, This one looks good. MESA_FORMAT_BGRA2101010_REV, I don't think you can have B with 2 bits. MESA_FORMAT_BGRA5999_REV, This one doesn't exist either and cannot be specified by TexImage. BGRAs and RGBAs were based on this from the spec: GL_INVALID_OPERATION is generated if type is one of GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_SHORT_4_4_4_4_REV, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_SHORT_1_5_5_5_REV, GL_UNSIGNED_INT_8_8_8_8, GL_UNSIGNED_INT_8_8_8_8_REV, GL_UNSIGNED_INT_10_10_10_2, GL_UNSIGNED_INT_2_10_10_10_REV, or GL_UNSIGNED_INT_5_9_9_9_REV, and format is neither GL_RGBA nor GL_BGRA. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Fri, Nov 22, 2013 at 12:36 PM, Marek Olšák mar...@gmail.com wrote: On Fri, Nov 15, 2013 at 9:58 PM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Nov 15, 2013 at 9:52 AM, Marek Olšák mar...@gmail.com wrote: I don't understand this and I don't think this is the right way to implement hw-accelerated TexImage. Some of the formats are unsupported by all hardware I know, others just don't make any sense (e.g. RGBA5999_REV). Please check out st_choose_matching_format. This is what the function comment says: /** * Given an OpenGL user-requested format and type, and swapBytes state, * return the format which exactly matches those parameters, so that * a memcpy-based transfer can be done. * * If no format is supported, return PIPE_FORMAT_NONE. */ My patch allows for similar functionality without using Gallium PIPE_FORMATs, which aren't supported in the i965 driver. RGPA5999_REV is there because it is used by a the glean test case pixelFormats. Having hardware support for the added formats is not relevant. Why is it not relevant? If there is no hardware support, adding those formats is a waste of time. Will the formats be unpacked by shaders or what? The MESA_* formats have only one purpose: to be used as OpenGL textures and renderbuffers. Yes, that's the nature of GPU based texture uploading. The driver generates a shader and the GPU does the necessary unpacking from a cached memcpy of the application texture and stuffs it into the final mip tree in its target tiled format. In order to do that, the driver must know the original format of the texture, which can't be done from the current choices of gl_formats, thus I'm adding more. Recently I've made some local improvements so the i965 driver that can produce more information from fewer gl_formats, thus I'll be posting an updated patch soon. If there's any question of the merits, here's an example - let's say the application uploads a GL_RGB format and asks for a GL_UNSIGNED_SHORT_5_6_5 internal format. Tests on Ivy Bridge show that when i965 uses the CPU to upload a 256 x 256 texture it does so at ~28 - 77 MB/sec, whereas when i965 uses the GPU, the same texture uploads at ~2400 MB/sec - including memcpy and all. Even the most basic cases with no repacking still show a 2x - 3x improvement because the GPU inherently handles tiling much faster. In general the overhead of the memcpy is easily absorbed by the fact that the graphics pipe doesn't have to stall while the CPU does the unpacking, tiling, and repacking in the single driver thread. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Fri, Nov 22, 2013 at 3:06 PM, Marek Olšák mar...@gmail.com wrote: On Fri, Nov 22, 2013 at 10:45 PM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Nov 22, 2013 at 12:36 PM, Marek Olšák mar...@gmail.com wrote: On Fri, Nov 15, 2013 at 9:58 PM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Nov 15, 2013 at 9:52 AM, Marek Olšák mar...@gmail.com wrote: I don't understand this and I don't think this is the right way to implement hw-accelerated TexImage. Some of the formats are unsupported by all hardware I know, others just don't make any sense (e.g. RGBA5999_REV). Please check out st_choose_matching_format. This is what the function comment says: /** * Given an OpenGL user-requested format and type, and swapBytes state, * return the format which exactly matches those parameters, so that * a memcpy-based transfer can be done. * * If no format is supported, return PIPE_FORMAT_NONE. */ My patch allows for similar functionality without using Gallium PIPE_FORMATs, which aren't supported in the i965 driver. RGPA5999_REV is there because it is used by a the glean test case pixelFormats. Having hardware support for the added formats is not relevant. Why is it not relevant? If there is no hardware support, adding those formats is a waste of time. Will the formats be unpacked by shaders or what? The MESA_* formats have only one purpose: to be used as OpenGL textures and renderbuffers. Yes, that's the nature of GPU based texture uploading. The driver generates a shader and the GPU does the necessary unpacking from a cached memcpy of the application texture and stuffs it into the final mip tree in its target tiled format. In order to do that, the driver must know the original format of the texture, which can't be done from the current choices of gl_formats, This is not true. Drivers have access to the parameters of TexImage and are allowed to do with them whatever they want, so they do know the exact format of user data. I don't see why you want new texture formats while all you need is a shader that can read raw bytes and do the unpacking. If you used a blit instead of a shader and your hardware had fixed-function circuitry to read the new formats, that would be an entirely different story. If I implemented shader-based unpacking, I would probably use a texture buffer object and common formats like R8UI, R16UI, R32UI, same for RG, RGB, and RGBA and also the signed variants, but not much else. How about let's forget the whole concept of GPU loading for a moment as that is only clouding the issue. As you said: the mesa_* formats have one purpose: to be used as OpenGL textures When I read the OGL spec for glTexImage2D it lists GL_BGR as a format. Yet looking at mesa_* formats there is only one BGR format represented: MESA_FORMAT_BGR888. There are 8 other valid OGL BGR textures that don't have MESA_FORMATs while all of the RGBs ones _are_ all there? ie. these are the OGL BGR formats that are missing: MESA_FORMAT_BGR_INT8, MESA_FORMAT_BGR_UINT8, MESA_FORMAT_BGR_INT16, MESA_FORMAT_BGR_UINT16, MESA_FORMAT_BGR_FLOAT16, MESA_FORMAT_BGR_INT32, MESA_FORMAT_BGR_UINT32, MESA_FORMAT_BGR_FLOAT32 There are also RGB, RGBA, and BGRA formats missing. Completeness seems like justification enough for their inclusion: /* Red Green Blue */ MESA_FORMAT_RGB233_REV, MESA_FORMAT_RGB10_REV, /* Red Green Blue Alpha */ MESA_FORMAT_RGBA1010102, MESA_FORMAT_RGBA2101010_REV, MESA_FORMAT_RGBA5999_REV, MESA_FORMAT_RGBA, MESA_FORMAT_RGBA_REV, MESA_FORMAT_RGBA1555_REV, /* Blue Green Red Alpha */ MESA_FORMAT_BGRA_INT8, MESA_FORMAT_BGRA_INT8_REV, MESA_FORMAT_BGRA_UINT8, MESA_FORMAT_BGRA_INT16, MESA_FORMAT_BGRA_UINT16, MESA_FORMAT_BGRA_FLOAT16, MESA_FORMAT_BGRA_INT32, MESA_FORMAT_BGRA_UINT32, MESA_FORMAT_BGRA_FLOAT32, MESA_FORMAT_BGRA, MESA_FORMAT_BGRA_REV, MESA_FORMAT_BGRA5551, MESA_FORMAT_BGRA1555_REV, MESA_FORMAT_BGRA1010102, MESA_FORMAT_BGRA2101010_REV, MESA_FORMAT_BGRA5999_REV, Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
On Fri, Nov 15, 2013 at 4:58 AM, Roland Scheidegger srol...@vmware.comwrote: On 11/14/2013 11:53 PM, Mark Mueller wrote: This and the subsequent patch are the first steps in adding support to load textures via GPU instead of CPU. This patch expands Mesa's gl_formats such that all GLUser format/type combinations are represented and thus can be communicated to lower levels within dri drivers. The new formats are source only thus they require no associated Texstore functions or render targets, therefore they are added _privately_ at the end of the gl_formats list. Do you really need 32bit normalized formats? I thought usually hw can't really deal with them anyway (because essentially you can't convert to floats without losing precision). I've gone back and forth on this. Ultimately I settled on the current setup because it followed how Mesa and dri drivers already handle other similar existing formats. The intent is to let the dri driver decide what the hardware can and can't deal with, the first step being communicating to the driver what format the GLUser has provided via a known mechanism. By the same logic, it's the driver's decision as to what format the texture takes when it gets to it's final resting place in video ram, thus the gl_format_info.DataType field isn't really relevant here. Some other minor nits inline, can't say if the approach in general makes sense (might make more sense to support these formats fully?). From my recent assessment, adding more formats with full support would offer more opportunities to picki target formats that require less CPU processing of the GLUser texture. My intent is to use the GPU for this, thus I'm not concerned for fully supporting the new cases by adding Texstore functions and render target info. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/main/formats.c | 441 ++ +++- src/mesa/main/formats.h | 70 +++- src/mesa/main/texformat.c | 271 src/mesa/main/texformat.h | 11 ++ 4 files changed, 789 insertions(+), 4 deletions(-) diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c index 07d2a72..2c32bba 100644 --- a/src/mesa/main/formats.c +++ b/src/mesa/main/formats.c @@ -76,7 +76,7 @@ struct gl_format_info * These must be in the same order as the MESA_FORMAT_* enums so that * we can do lookups without searching. */ -static struct gl_format_info format_info[MESA_FORMAT_COUNT] = +static struct gl_format_info format_info[MESA_PRIVATE_FORMAT_COUNT] = { { MESA_FORMAT_NONE,/* Name */ @@ -1572,7 +1572,7 @@ static struct gl_format_info format_info[MESA_FORMAT_COUNT] = }, { MESA_FORMAT_RGB9_E5_FLOAT, - MESA_FORMAT_RGB9_E5, + MESA_FORMAT_RGB9_E5_FLOAT, GL_RGB, GL_FLOAT, 9, 9, 9, 0, @@ -1763,6 +1763,440 @@ static struct gl_format_info format_info[MESA_FORMAT_COUNT] = 0, 0, 0, 0, 0, 1, 1, 16 }, + + /* +* Formats to exactly represent format/type combinations for OGLUser provided +* textures. These formats are required only for caching of an unaltered copy of +* the application texture prior to passing them to the GPU, where they are +* processed to the final internalFormat and miptree location. Texstore functions +* do not apply and these are not intended to be used as render targets. +* Listing is based on the order presented in the glTexImage2D spec. +*/ + /* Red Solo - !cup */ + { + MESA_FORMAT_R32, + MESA_FORMAT_R32, + GL_RED, + GL_UNSIGNED_NORMALIZED, + 16, 0, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 2 Should probably 32bits (and 4 bytes) if it's named R32. Definitely should. Thanks. + }, + { + MESA_FORMAT_SIGNED_R32, + MESA_FORMAT_SIGNED_R32, + GL_RED, + GL_SIGNED_NORMALIZED, + 32, 0, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 4 + }, + + /* Red Green - !show */ + { + MESA_FORMAT_SIGNED_RG88, + MESA_FORMAT_SIGNED_RG88, + GL_RG, + GL_SIGNED_NORMALIZED, + 8, 8, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 2 + }, + { + MESA_FORMAT_SIGNED_RG1616, + MESA_FORMAT_SIGNED_RG1616, + GL_RG, + GL_SIGNED_NORMALIZED, + 16, 16, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 4 + }, + { + MESA_FORMAT_RG3232, + MESA_FORMAT_RG3232, + GL_RG, + GL_UNSIGNED_NORMALIZED, + 32, 32, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 8 + }, + { + MESA_FORMAT_SIGNED_RG3232, + MESA_FORMAT_SIGNED_RG3232, + GL_RG, + GL_SIGNED_NORMALIZED, + 32, 32, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 8 + }, + + /* Red Green Blue */ + { + MESA_FORMAT_SIGNED_RGB888, + MESA_FORMAT_SIGNED_RGB888, + GL_RGB, + GL_SIGNED_NORMALIZED, + 8, 8
[Mesa-dev] [PATCH 1/2] mesa: Add gl_formats to cover all GLUser provided format/type combinations
This and the subsequent patch are the first steps in adding support to load textures via GPU instead of CPU. This patch expands Mesa's gl_formats such that all GLUser format/type combinations are represented and thus can be communicated to lower levels within dri drivers. The new formats are source only thus they require no associated Texstore functions or render targets, therefore they are added _privately_ at the end of the gl_formats list. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/main/formats.c | 441 +- src/mesa/main/formats.h | 70 +++- src/mesa/main/texformat.c | 271 src/mesa/main/texformat.h | 11 ++ 4 files changed, 789 insertions(+), 4 deletions(-) diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c index 07d2a72..2c32bba 100644 --- a/src/mesa/main/formats.c +++ b/src/mesa/main/formats.c @@ -76,7 +76,7 @@ struct gl_format_info * These must be in the same order as the MESA_FORMAT_* enums so that * we can do lookups without searching. */ -static struct gl_format_info format_info[MESA_FORMAT_COUNT] = +static struct gl_format_info format_info[MESA_PRIVATE_FORMAT_COUNT] = { { MESA_FORMAT_NONE,/* Name */ @@ -1572,7 +1572,7 @@ static struct gl_format_info format_info[MESA_FORMAT_COUNT] = }, { MESA_FORMAT_RGB9_E5_FLOAT, - MESA_FORMAT_RGB9_E5, + MESA_FORMAT_RGB9_E5_FLOAT, GL_RGB, GL_FLOAT, 9, 9, 9, 0, @@ -1763,6 +1763,440 @@ static struct gl_format_info format_info[MESA_FORMAT_COUNT] = 0, 0, 0, 0, 0, 1, 1, 16 }, + + /* +* Formats to exactly represent format/type combinations for OGLUser provided +* textures. These formats are required only for caching of an unaltered copy of +* the application texture prior to passing them to the GPU, where they are +* processed to the final internalFormat and miptree location. Texstore functions +* do not apply and these are not intended to be used as render targets. +* Listing is based on the order presented in the glTexImage2D spec. +*/ + /* Red Solo - !cup */ + { + MESA_FORMAT_R32, + MESA_FORMAT_R32, + GL_RED, + GL_UNSIGNED_NORMALIZED, + 16, 0, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 2 + }, + { + MESA_FORMAT_SIGNED_R32, + MESA_FORMAT_SIGNED_R32, + GL_RED, + GL_SIGNED_NORMALIZED, + 32, 0, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 4 + }, + + /* Red Green - !show */ + { + MESA_FORMAT_SIGNED_RG88, + MESA_FORMAT_SIGNED_RG88, + GL_RG, + GL_SIGNED_NORMALIZED, + 8, 8, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 2 + }, + { + MESA_FORMAT_SIGNED_RG1616, + MESA_FORMAT_SIGNED_RG1616, + GL_RG, + GL_SIGNED_NORMALIZED, + 16, 16, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 4 + }, + { + MESA_FORMAT_RG3232, + MESA_FORMAT_RG3232, + GL_RG, + GL_UNSIGNED_NORMALIZED, + 32, 32, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 8 + }, + { + MESA_FORMAT_SIGNED_RG3232, + MESA_FORMAT_SIGNED_RG3232, + GL_RG, + GL_SIGNED_NORMALIZED, + 32, 32, 0, 0, + 0, 0, 0, 0, 0, + 1, 1, 8 + }, + + /* Red Green Blue */ + { + MESA_FORMAT_SIGNED_RGB888, + MESA_FORMAT_SIGNED_RGB888, + GL_RGB, + GL_SIGNED_NORMALIZED, + 8, 8, 8, 0, + 0, 0, 0, 0, 0, + 1, 1, 3 + }, + { + MESA_FORMAT_RGB161616, + MESA_FORMAT_RG161616, + GL_RGB, + GL_UNSIGNED_NORMALIZED, + 16, 16, 16, 0, + 0, 0, 0, 0, 0, + 1, 1, 6 + }, + { + MESA_FORMAT_SIGNED_RGB161616, + MESA_FORMAT_SIGNED_RGB161616, + GL_RGB, + GL_SIGNED_NORMALIZED, + 16, 16, 16, 0, + 0, 0, 0, 0, 0, + 1, 1, 6 + }, + { + MESA_FORMAT_RGB323232, + MESA_FORMAT_RGB323232, + GL_RGB, + GL_UNSIGNED_NORMALIZED, + 32, 32, 32, 0, + 0, 0, 0, 0, 0, + 1, 1, 12 + }, + { + MESA_FORMAT_SIGNED_RGB323232, + MESA_FORMAT_SIGNED_RGB323232, + GL_RGB, + GL_SIGNED_NORMALIZED, + 32, 32, 32, 0, + 0, 0, 0, 0, 0, + 1, 1, 12 + }, + { + MESA_FORMAT_RGB233_REV, + MESA_FORMAT_RGB233_REV, + GL_RGB, + GL_UNSIGNED_NORMALIZED, + 3, 3, 2, 0, + 0, 0, 0, 0, 0, + 1, 1, 1 + }, + { + MESA_FORMAT_RGB10_REV, + MESA_FORMAT_RGB10_REV, + GL_RGB, + GL_UNSIGNED_NORMALIZED, + 11, 11, 10, 0, + 0, 0, 0, 0, 0, + 1, 1, 4 + }, + + /* Blue Green Red */ + { + MESA_FORMAT_SIGNED_BGR888, + MESA_FORMAT_SIGNED_BGR888, + GL_RGB, + GL_SIGNED_NORMALIZED, + 8, 8, 8, 0, + 0, 0, 0, 0, 0, + 1, 1, 3 + }, + { + MESA_FORMAT_BGR161616, + MESA_FORMAT_BGR161616, + GL_RGB, + GL_UNSIGNED_NORMALIZED, + 16, 16, 16, 0, + 0, 0, 0, 0, 0, + 1, 1, 6
[Mesa-dev] [i965] Allow blorp to make decisions about the formats that it supports where it can
This patch consolidates the decision about formats that blorp_blt does and does not support to blorp instead of leaving that decision to callers. This opens the door to adding more functionality to blorp, including support for using GPU acceleration of processing and loading textures. This patch fixes piglit generated_tests/spec/glsl-1.50/compiler/- built-in-functions/op-div-mat4x3-float.geom. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_blorp.h | 8 +- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 149 ++ src/mesa/drivers/dri/i965/brw_state.h | 5 +- src/mesa/drivers/dri/i965/brw_surface_formats.c | 26 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +- src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 4 +- 6 files changed, 131 insertions(+), 73 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h index 85bf099..5344851 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.h +++ b/src/mesa/drivers/dri/i965/brw_blorp.h @@ -34,8 +34,7 @@ struct brw_context; extern C { #endif -void -brw_blorp_blit_miptrees(struct brw_context *brw, +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, struct intel_mipmap_tree *dst_mt, @@ -356,8 +355,13 @@ public: virtual uint32_t get_wm_prog(struct brw_context *brw, brw_blorp_prog_data **prog_data) const; + bool is_valid() { + return valid_parameters; + } + private: brw_blorp_blit_prog_key wm_prog_key; + bool valid_parameters; }; /** diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp index d54b926..00461a4 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp @@ -123,7 +123,7 @@ find_miptree(GLbitfield buffer_bit, struct intel_renderbuffer *irb) return mt; } -void +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, @@ -135,16 +135,6 @@ brw_blorp_blit_miptrees(struct brw_context *brw, float dst_x1, float dst_y1, GLenum filter, bool mirror_x, bool mirror_y) { - /* Get ready to blit. This includes depth resolving the src and dst -* buffers if necessary. Note: it's not necessary to do a color resolve on -* the destination buffer because we use the standard render path to render -* to destination color buffers, and the standard render path is -* fast-color-aware. -*/ - intel_miptree_resolve_color(brw, src_mt); - intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); - intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); - DBG(%s from %s mt %p %d %d (%f,%f) (%f,%f) to %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n, __FUNCTION__, @@ -162,12 +152,28 @@ brw_blorp_blit_miptrees(struct brw_context *brw, dst_x0, dst_y0, dst_x1, dst_y1, filter, mirror_x, mirror_y); + + if (!params.is_valid()) { + return false; + } + + /* Get ready to blit. This includes depth resolving the src and dst +* buffers if necessary. Note: it's not necessary to do a color resolve on +* the destination buffer because we use the standard render path to render +* to destination color buffers, and the standard render path is +* fast-color-aware. +*/ + intel_miptree_resolve_color(brw, src_mt); + intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); + intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); + brw_blorp_exec(brw, params); intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level, dst_layer); + return true; } -static void +static bool do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit, struct intel_renderbuffer *src_irb, struct intel_renderbuffer *dst_irb, @@ -180,14 +186,17 @@ do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit, struct intel_mipmap_tree *dst_mt = find_miptree(buffer_bit, dst_irb); /* Do the blit */ - brw_blorp_blit_miptrees(brw, - src_mt, src_irb-mt_level, src_irb-mt_layer, - dst_mt, dst_irb-mt_level, dst_irb-mt_layer, - srcX0, srcY0, srcX1, srcY1, - dstX0, dstY0, dstX1, dstY1, - filter, mirror_x, mirror_y); + if (!brw_blorp_blit_miptrees(brw, +src_mt, src_irb-mt_level, src_irb-mt_layer, +dst_mt, dst_irb
[Mesa-dev] [PATCH 2/2] i965: Allow blorp to make decisions about the formats that it supports where it can
fixed subject line On Thu, Nov 14, 2013 at 4:15 PM, Mark Mueller markkmuel...@gmail.comwrote: This patch consolidates the decision about formats that blorp_blt does and does not support to blorp instead of leaving that decision to callers. This opens the door to adding more functionality to blorp, including support for using GPU acceleration of processing and loading textures. This patch fixes piglit generated_tests/spec/glsl-1.50/compiler/- built-in-functions/op-div-mat4x3-float.geom. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_blorp.h | 8 +- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 149 ++ src/mesa/drivers/dri/i965/brw_state.h | 5 +- src/mesa/drivers/dri/i965/brw_surface_formats.c | 26 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +- src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 4 +- 6 files changed, 131 insertions(+), 73 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h index 85bf099..5344851 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.h +++ b/src/mesa/drivers/dri/i965/brw_blorp.h @@ -34,8 +34,7 @@ struct brw_context; extern C { #endif -void -brw_blorp_blit_miptrees(struct brw_context *brw, +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, struct intel_mipmap_tree *dst_mt, @@ -356,8 +355,13 @@ public: virtual uint32_t get_wm_prog(struct brw_context *brw, brw_blorp_prog_data **prog_data) const; + bool is_valid() { + return valid_parameters; + } + private: brw_blorp_blit_prog_key wm_prog_key; + bool valid_parameters; }; /** diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp index d54b926..00461a4 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp @@ -123,7 +123,7 @@ find_miptree(GLbitfield buffer_bit, struct intel_renderbuffer *irb) return mt; } -void +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, @@ -135,16 +135,6 @@ brw_blorp_blit_miptrees(struct brw_context *brw, float dst_x1, float dst_y1, GLenum filter, bool mirror_x, bool mirror_y) { - /* Get ready to blit. This includes depth resolving the src and dst -* buffers if necessary. Note: it's not necessary to do a color resolve on -* the destination buffer because we use the standard render path to render -* to destination color buffers, and the standard render path is -* fast-color-aware. -*/ - intel_miptree_resolve_color(brw, src_mt); - intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); - intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); - DBG(%s from %s mt %p %d %d (%f,%f) (%f,%f) to %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n, __FUNCTION__, @@ -162,12 +152,28 @@ brw_blorp_blit_miptrees(struct brw_context *brw, dst_x0, dst_y0, dst_x1, dst_y1, filter, mirror_x, mirror_y); + + if (!params.is_valid()) { + return false; + } + + /* Get ready to blit. This includes depth resolving the src and dst +* buffers if necessary. Note: it's not necessary to do a color resolve on +* the destination buffer because we use the standard render path to render +* to destination color buffers, and the standard render path is +* fast-color-aware. +*/ + intel_miptree_resolve_color(brw, src_mt); + intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); + intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); + brw_blorp_exec(brw, params); intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level, dst_layer); + return true; } -static void +static bool do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit, struct intel_renderbuffer *src_irb, struct intel_renderbuffer *dst_irb, @@ -180,14 +186,17 @@ do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit, struct intel_mipmap_tree *dst_mt = find_miptree(buffer_bit, dst_irb); /* Do the blit */ - brw_blorp_blit_miptrees(brw, - src_mt, src_irb-mt_level, src_irb-mt_layer, - dst_mt, dst_irb-mt_level, dst_irb-mt_layer, - srcX0, srcY0, srcX1, srcY1, - dstX0, dstY0, dstX1, dstY1
Re: [Mesa-dev] [PATCH 2/2] i965: Allow blorp to make decisions about the formats that it supports where it can
On Thu, Nov 14, 2013 at 5:18 PM, Chris Forbes chr...@ijw.co.nz wrote: Why does this affect that piglit test? Oh wait, it was patch 1/2 that fixed that test. Do I still need to answer the question? :) 1/2 made no significant functional changes so I don't have an explanation, but I will do some checking and get back -- Chris On Fri, Nov 15, 2013 at 2:01 PM, Mark Mueller markkmuel...@gmail.com wrote: fixed subject line On Thu, Nov 14, 2013 at 4:15 PM, Mark Mueller markkmuel...@gmail.com wrote: This patch consolidates the decision about formats that blorp_blt does and does not support to blorp instead of leaving that decision to callers. This opens the door to adding more functionality to blorp, including support for using GPU acceleration of processing and loading textures. This patch fixes piglit generated_tests/spec/glsl-1.50/compiler/- built-in-functions/op-div-mat4x3-float.geom. Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_blorp.h | 8 +- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 149 ++ src/mesa/drivers/dri/i965/brw_state.h | 5 +- src/mesa/drivers/dri/i965/brw_surface_formats.c | 26 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +- src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 4 +- 6 files changed, 131 insertions(+), 73 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h index 85bf099..5344851 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.h +++ b/src/mesa/drivers/dri/i965/brw_blorp.h @@ -34,8 +34,7 @@ struct brw_context; extern C { #endif -void -brw_blorp_blit_miptrees(struct brw_context *brw, +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, struct intel_mipmap_tree *dst_mt, @@ -356,8 +355,13 @@ public: virtual uint32_t get_wm_prog(struct brw_context *brw, brw_blorp_prog_data **prog_data) const; + bool is_valid() { + return valid_parameters; + } + private: brw_blorp_blit_prog_key wm_prog_key; + bool valid_parameters; }; /** diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp index d54b926..00461a4 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp @@ -123,7 +123,7 @@ find_miptree(GLbitfield buffer_bit, struct intel_renderbuffer *irb) return mt; } -void +bool brw_blorp_blit_miptrees(struct brw_context *brw, struct intel_mipmap_tree *src_mt, unsigned src_level, unsigned src_layer, @@ -135,16 +135,6 @@ brw_blorp_blit_miptrees(struct brw_context *brw, float dst_x1, float dst_y1, GLenum filter, bool mirror_x, bool mirror_y) { - /* Get ready to blit. This includes depth resolving the src and dst -* buffers if necessary. Note: it's not necessary to do a color resolve on -* the destination buffer because we use the standard render path to render -* to destination color buffers, and the standard render path is -* fast-color-aware. -*/ - intel_miptree_resolve_color(brw, src_mt); - intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); - intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); - DBG(%s from %s mt %p %d %d (%f,%f) (%f,%f) to %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n, __FUNCTION__, @@ -162,12 +152,28 @@ brw_blorp_blit_miptrees(struct brw_context *brw, dst_x0, dst_y0, dst_x1, dst_y1, filter, mirror_x, mirror_y); + + if (!params.is_valid()) { + return false; + } + + /* Get ready to blit. This includes depth resolving the src and dst +* buffers if necessary. Note: it's not necessary to do a color resolve on +* the destination buffer because we use the standard render path to render +* to destination color buffers, and the standard render path is +* fast-color-aware. +*/ + intel_miptree_resolve_color(brw, src_mt); + intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer); + intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer); + brw_blorp_exec(brw, params); intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level, dst_layer); + return true; } -static void +static bool do_blorp_blit(struct brw_context *brw, GLbitfield buffer_bit, struct intel_renderbuffer *src_irb
Re: [Mesa-dev] [PATCH] i965: Fast texture upload now supports all levels
Hi Courtney, I've been doing similar work but using Xenotic as the benchmark. Here is how I've been estimating upload times: First I made a rough determination of a texture count, like 300, as a metric. In intelTexImage I use clock_gettime to determine the elapsed time between the loading of texture 0 and texture 300. Then I use _mesa_debug to output the results to a log file by setting these env variables: export MESA_DEBUG=1; export MESA_LOG_FILE=`pwd`/mesa.log Cheers, Mark On Mon, Oct 14, 2013 at 9:00 AM, Courtney Goeltzenleuchter court...@lunarg.com wrote: Does anyone know of a test that measures frame 0 time? Or texture upload speed? For Smokin' Guns, I tried measuring the overall time, but an improved frame 0 time has difficulty standing out of a 2607 frame test. I may have to create something. Suggestions for an appropriate framework? Thanks, Courtney On Mon, Oct 14, 2013 at 8:32 AM, Chad Versace chad.vers...@linux.intel.com wrote: On 10/13/2013 08:33 PM, Ian Romanick wrote: On 10/13/2013 01:50 PM, Frank Henigman wrote: On Fri, Oct 11, 2013 at 10:00 PM, Chad Versace chad.vers...@linux.intel.com wrote: On 10/11/2013 10:17 AM, Courtney Goeltzenleuchter wrote: Support all levels of a supported texture format. --- src/mesa/drivers/dri/i965/**intel_tex_subimage.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/**intel_tex_subimage.c b/src/mesa/drivers/dri/i965/**intel_tex_subimage.c index 4aec05d..5e46760 100644 --- a/src/mesa/drivers/dri/i965/**intel_tex_subimage.c +++ b/src/mesa/drivers/dri/i965/**intel_tex_subimage.c @@ -541,14 +541,13 @@ intel_texsubimage_tiled_**memcpy(struct gl_context * ctx, uint32_t cpp; mem_copy_fn mem_copy = NULL; - /* This fastpath is restricted to specific texture types: level 0 of + /* This fastpath is restricted to specific texture types: * a 2D BGRA, RGBA, L8 or A8 texture. It could be generalized to support * more types. */ if (!brw-has_llc || type != GL_UNSIGNED_BYTE || texImage-TexObject-Target != GL_TEXTURE_2D || - texImage-Level != 0 || pixels == NULL || _mesa_is_bufferobj(packing-**BufferObj) || packing-Alignment 4 || @@ -616,6 +615,16 @@ intel_texsubimage_tiled_**memcpy(struct gl_context * ctx, DBG(%s: level=%d offset=(%d,%d) (w,h)=(%d,%d)\n, __FUNCTION__, texImage-Level, xoffset, yoffset, width, height); + /* Adjust x and y offset based on miplevel +*/ + if (texImage-Level) { + GLuint xlevel, ylevel; + intel_miptree_get_image_**offset(image-mt, texImage-Level, 0, + xlevel, ylevel); + xoffset += xlevel; + yoffset += ylevel; + } + linear_to_tiled( xoffset * cpp, (xoffset + width) * cpp, yoffset, yoffset + height, Usually when we commit performance patches like this, we state in the commit message what the observed relative performance gain. What gain did you see? Hardware? Benchmark? Kernel version? How many runs? We could quote from my patch, as this is just opening more paths into that code. Or do you think this calls for different testing? I think what Chad is asking is whether there's some information like Improves load time of application XYZ 12.3+4.5% or similar. In the past, we've had problems with patches that just make vague claims of improves performance when we later find critical bugs in those patches... can we just revert the code, or is it going to run the performance of... something? For reference, see commit 329cd6a9b and this thread from mesa-dev: http://lists.freedesktop.org/**archives/mesa-dev/2013-June/**040811.htmlhttp://lists.freedesktop.org/archives/mesa-dev/2013-June/040811.html Ian read my mind correctly. The commit message should say Improves XYZ of application ABC by 10.3+-1.2%, as well as state the hardware at a minimum, and kernel version too if you're feeling gracious. In the future, if someone discover that this patch introduces a bug, the commit message's performance claim will prevent that someone from simply reverting the code. -- Courtney Goeltzenleuchter LunarG ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/hsw: compute DDX in a subspan based only on top row
Wow, it's not often we as developers get to attribute an unexplained performance improvement to silicon. I'm happy to say I guessed wrong here. Did this start specifically with Haswell and is it likely this will persist in future gen hardware - more specifically, what's the proper test by the driver for availability of this feature in hardware? Finally, were any other silicon improvements revealed besides what Chai-I was able to expose? Mark On Fri, Sep 20, 2013 at 12:51 PM, Kenneth Graunke kenn...@whitecape.orgwrote: On 09/20/2013 08:30 AM, Ian Romanick wrote: On 09/20/2013 09:50 AM, Paul Berry wrote: [snip] Since the SAMPLER_MODE setting allows us to trade off quality vs performance, we're also interested to know whether a value less than 0x1f is sufficient to produce the performance improvement in Xonotic--it would be nice if we could find a sweet spot for this setting that produces the performance improvement we need without sacrificing too much quality. How about if we just give a driconf option to adjust it. Then gamers can make their own choice. For applications where it know it makes a big difference, we can provide a default non-0 value in the system driconf. Because you can't (yet) program registers from userspace unless you're root. I would like to use the same default value as Windows. I'm fine with making it tunable beyond that. --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/hsw: compute DDX in a subspan based only on top row
On Mon, Sep 16, 2013 at 1:31 AM, Chia-I Wu olva...@gmail.com wrote: On Mon, Sep 16, 2013 at 4:12 PM, Chia-I Wu olva...@gmail.com wrote: On Mon, Sep 16, 2013 at 3:50 AM, Mark Mueller markkmuel...@gmail.com wrote: On Fri, Sep 13, 2013 at 2:15 PM, Paul Berry stereotype...@gmail.com wrote: On 12 September 2013 22:06, Chia-I Wu olva...@gmail.com wrote: From: Chia-I Wu o...@lunarg.com This scenario is where I'd place my bets, especially given that the numbers are based on Xonotic. I benchmarked this patch using Xonotic on Bay Trail as is and by replacing !brw-is_haswell with !brw-is_baytrail. With ultra and ultimate levels at medium and high resolutions, the results were all essentially the same at comparable resolutions and quality levels. Isn't Bay Trail based on Ivy Bridge? For Bay Trail, this might help you http://lists.freedesktop.org/archives/mesa-dev/2013-September/044288.html if you are interested. Testing with Bay Trail shows no performance improvement with this patch. Most likely there are one or more CPU bottlenecks on Bay Tail that hide a majority of the performance gains of this change. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/hsw: compute DDX in a subspan based only on top row
On Fri, Sep 13, 2013 at 2:15 PM, Paul Berry stereotype...@gmail.com wrote: On 12 September 2013 22:06, Chia-I Wu olva...@gmail.com wrote: From: Chia-I Wu o...@lunarg.com Consider only the top-left and top-right pixels to approximate DDX in a 2x2 subspan, unless the application or the user requests a more accurate approximation. This results in a less accurate approximation. However, it improves the performance of Xonotic with Ultra settings by 24.3879% +/- 0.832202% (at 95.0% confidence) on Haswell. No noticeable image quality difference observed. No piglit gpu.tests regressions (tested with v1) I failed to come up with an explanation for the performance difference, as the change does not affect Ivy Bridge. If anyone has the insight, please kindly enlighten me. Performance differences may also be observed on other games that call textureGrad and dFdx. v2: Honor GL_FRAGMENT_SHADER_DERIVATIVE_HINT and add a drirc option. Update comments. I'm not entirely comfortable making a change that has a known negative impact on computational accuracy (even one that leads to such an impressive performance improvement) when we don't have any theories as to why the performance improvement happens, or why the improvement doesn't apply to Ivy Bridge. In my experience, making changes to the codebase without understanding why they improve things almost always leads to improvements that are brittle, since it's likely that the true source of the improvement is a coincidence that will be wiped out by some future change (or won't be relevant to client programs other than this particular benchmark). Having a theory as to why the performance improvement happens would help us be confident that we're applying the right fix under the right circumstances. There's another angle to approach this and that is to develop a simple test case that will show the different results across a range of computational accuracy and run the test on proprietary drivers for the same hardware to determine what settings they are using. For example, here's one theory as to why we might be seeing an improvement: perhaps Haswell's sample_d processing is smart enough to realize that when all the gradient values within a sub-span are the same, that means that all of the sampling for the sub-span will come from the same LOD, and that allows it to short-cut some expensive step in the LOD calculation. Perhaps the same improvement isn't seen on Ivy Bridge because Ivy Bridge's sample_d processing logic is less sophisticated, so it's unable to perform the optimization. If this is the case, then conditioning the optimization on brw-is_haswell (as you've done) makes sense. Another possible explanation for the Haswell vs Ivy Bridge difference is that perhaps Ivy Bridge, being a lower-performing chip, has other bottlenecks that make the optimization irrelevant for this particular benchmark, but potentially still useful for other benchmarks. For instance, maybe when this benchmark executes on Ivy Bridge, the texture that's being sampled from is located in sufficiently distant memory that optimizing the sample_d's memory accesses makes no difference, since the bottleneck is the speed with which the texture can be read into cache, rather than the speed of operation of sample_d. If this explanation is correct, then it might be worth applying the optimization to both Ivy Bridge and Haswell (and perhaps Sandy Bridge as well), since it might conceivably benefit those other chips when running applications that place less cache pressure on the chip. This scenario is where I'd place my bets, especially given that the numbers are based on Xonotic. I benchmarked this patch using Xonotic on Bay Trail as is and by replacing !brw-is_haswell with !brw-is_baytrail. With ultra and ultimate levels at medium and high resolutions, the results were all essentially the same at comparable resolutions and quality levels. I don't see any justification to tie this change to just Haswell hardware. There's all sorts of reasons why doing that sounds like a big mistake. In fact, another _explanation_ to add to your list is maybe there's another is_haswell test elsewhere in the driver that is responsible for the performance anomaly. Another possibile explanation is that Haswell has a bug in its sample_d logic which causes it to be slow under some conditions, and this lower-accuracy DDX computation happens to work around it. If that's the case, we might want to consider not using sample_d at all on Haswell, and instead calculating the LOD in the shader and using sample_l instead. If this is the correct explanation, then that might let us have faster performance without sacrificing DDX accuracy. A final possible explanation for the performance improvement is that perhaps for some reason sample_d performs more optimally when the DDX and DDY computations have similar accuracies to each other. Before your
Re: [Mesa-dev] [RFC PATCH] i965: Allow C++ type safety in the use of enum brw_urb_write_flags.
This is a nice improvement over the explicit cast, which is how I've always done it in the past - which is the ugly part of an otherwise great method for flags. Also I use a lot with enum for clearing bits. On Fri, Aug 23, 2013 at 3:18 PM, Paul Berry stereotype...@gmail.com wrote: (From a suggestion by Francisco Jerez) If an enum represents a bitfield of flags, e.g.: enum E { A = 1, B = 2, C = 4, D = 8, }; then C++ normally prohibits statements like this: enum E x = A | B; because A and B are implicitly converted to ints before OR-ing them, and an int can't be stored in an enum without a type cast. C, on the other hand, allows an int to be implicitly converted to an enum without casting. In the past we've dealt with this situation by storing flag bitfields as ints. This avoids ugly casting at the expense of some type safety that C++ would normally have offered (e.g. we get no warning if we accidentally use the wrong enum type). However, we can get the best of both worlds if we override the | operator. The ugly casting is confined to the operator overload, and we still get the benefit of C++ making sure we don't use the wrong enum type. The disadvantages are that (a) we need an explicit enum value for 0, and (b) we can't use related operators like |= unless we define additional overloads. So what do folks think? Is it worth it? Cc: Francisco Jerez curroje...@riseup.net --- src/mesa/drivers/dri/i965/brw_clip.h | 2 +- src/mesa/drivers/dri/i965/brw_clip_util.c | 2 +- src/mesa/drivers/dri/i965/brw_eu.h | 16 +++- src/mesa/drivers/dri/i965/brw_eu_emit.c| 4 ++-- src/mesa/drivers/dri/i965/brw_sf_emit.c| 12 src/mesa/drivers/dri/i965/brw_vec4.h | 2 +- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 3 ++- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_clip.h b/src/mesa/drivers/dri/i965/brw_clip.h index 5af0ad3..41f5c75 100644 --- a/src/mesa/drivers/dri/i965/brw_clip.h +++ b/src/mesa/drivers/dri/i965/brw_clip.h @@ -173,7 +173,7 @@ void brw_clip_init_planes( struct brw_clip_compile *c ); void brw_clip_emit_vue(struct brw_clip_compile *c, struct brw_indirect vert, - unsigned flags, + enum brw_urb_write_flags flags, GLuint header); void brw_clip_kill_thread(struct brw_clip_compile *c); diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c b/src/mesa/drivers/dri/i965/brw_clip_util.c index d5c50d7..24d053e 100644 --- a/src/mesa/drivers/dri/i965/brw_clip_util.c +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c @@ -313,7 +313,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c, void brw_clip_emit_vue(struct brw_clip_compile *c, struct brw_indirect vert, - unsigned flags, + enum brw_urb_write_flags flags, GLuint header) { struct brw_compile *p = c-func; diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h index 9053ea2..069b223 100644 --- a/src/mesa/drivers/dri/i965/brw_eu.h +++ b/src/mesa/drivers/dri/i965/brw_eu.h @@ -229,6 +229,8 @@ void brw_set_dp_write_message(struct brw_compile *p, GLuint send_commit_msg); enum brw_urb_write_flags { + BRW_URB_WRITE_NO_FLAGS = 0, + /** * Causes a new URB entry to be allocated, and its address stored in the * destination register (gen 7). @@ -271,11 +273,23 @@ enum brw_urb_write_flags { BRW_URB_WRITE_ALLOCATE | BRW_URB_WRITE_COMPLETE, }; +#ifdef __cplusplus +/** + * Allow brw_urb_write_flags enums to be ORed together (normally C++ wouldn't + * allow this without a type cast). + */ +inline enum brw_urb_write_flags +operator|(enum brw_urb_write_flags x, enum brw_urb_write_flags y) +{ + return (enum brw_urb_write_flags) ((int) x | (int) y); +} +#endif + void brw_urb_WRITE(struct brw_compile *p, struct brw_reg dest, GLuint msg_reg_nr, struct brw_reg src0, - unsigned flags, + enum brw_urb_write_flags flags, GLuint msg_length, GLuint response_length, GLuint offset, diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c index b55b57e..ecf8597 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c @@ -515,7 +515,7 @@ static void brw_set_ff_sync_message(struct brw_compile *p, static void brw_set_urb_message( struct brw_compile *p, struct brw_instruction *insn, - unsigned flags, + enum brw_urb_write_flags flags,
Re: [Mesa-dev] [PATCH] i965: STATIC_ASSERT that there aren't too many BRW_NEW_* flags.
When the time comes, are there any concerns with using a 64 bit type, like portability? 64 bits for flags would be useful for something that I'm looking into and I'm curious how much pain that could cause. On Sun, Aug 18, 2013 at 10:58 AM, Ian Romanick i...@freedesktop.org wrote: On 08/18/2013 09:34 AM, Paul Berry wrote: We are getting close to the maximum number of BRW_NEW_* bits that can be stored in brw-state.dirty.brw without overflowing 32 bits, and geometry shaders are going to add more. Add a STATIC_ASSERT so that we will be alerted when we need to switch to 64 bits. Good call. Reviewed-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/drivers/dri/i965/brw_**context.c | 5 + src/mesa/drivers/dri/i965/brw_**context.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/mesa/drivers/dri/i965/**brw_context.c b/src/mesa/drivers/dri/i965/**brw_context.c index 11d438b..44a35d1 100644 --- a/src/mesa/drivers/dri/i965/**brw_context.c +++ b/src/mesa/drivers/dri/i965/**brw_context.c @@ -448,6 +448,11 @@ brwCreateContext(int api, brw-state.dirty.mesa = ~0; brw-state.dirty.brw = ~0; + /* Make sure that brw-state.dirty.brw has enough bits to hold all possible +* dirty flags. +*/ + STATIC_ASSERT(BRW_NUM_STATE_**BITS = 8 * sizeof(brw-state.dirty.brw)); + brw-emit_state_always = 0; brw-batch.need_workaround_**flush = true; diff --git a/src/mesa/drivers/dri/i965/**brw_context.h b/src/mesa/drivers/dri/i965/**brw_context.h index 74e38f1..dbad507 100644 --- a/src/mesa/drivers/dri/i965/**brw_context.h +++ b/src/mesa/drivers/dri/i965/**brw_context.h @@ -155,6 +155,7 @@ enum brw_state_id { BRW_STATE_UNIFORM_BUFFER, BRW_STATE_META_IN_PROGRESS, BRW_STATE_INTERPOLATION_MAP, + BRW_NUM_STATE_BITS }; #define BRW_NEW_URB_FENCE (1 BRW_STATE_URB_FENCE) __**_ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/**mailman/listinfo/mesa-devhttp://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [i965] i965/draw: Move constant formation outside of for loop and use an enum.
On Tue, Aug 6, 2013 at 12:31 PM, Ian Romanick i...@freedesktop.org wrote: On 08/06/2013 12:10 PM, mmueller wrote:Signed-off-by: mmueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_**draw.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/mesa/drivers/dri/i965/**brw_draw.c b/src/mesa/drivers/dri/i965/**brw_draw.c index 6170d07..e11d0d8 100644 --- a/src/mesa/drivers/dri/i965/**brw_draw.c +++ b/src/mesa/drivers/dri/i965/**brw_draw.c @@ -367,6 +367,15 @@ static bool brw_try_draw_prims( struct gl_context *ctx, bool retval = true; GLuint i; bool fail_next = false; + enum { + estimated_max_prim_size = + 512 + /* batchbuffer commands */ + ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color + + 1024 + /* gen6 VS push constants */ + 1024 + /* gen6 WM push constants */ + 512 /* misc. pad */ + }; + I think this would be better as 'const int estimated_max_prim_size = ...'. Using an enum is mostly the same (but also enforces that it's a compile-time constant), but it looks weird. :) OK. A new patch is on the way *sigh*, I knew this day was coming for the last 15 years, but I don't feel that was enough time to prepare. Now I'm forced to come out of the closet - I'm a lifelong member of PETA*. All too often I clearly communicate something to the compiler yet it still goes off and does something else. Before I started using enums to replace #defines and const PODs in my code, I too thought enums were weird and ugly. After time I came to realized that it was an effective means to get the compiler to do some useful things. After a while they don't look so weird. Obviously in this case it's of no concern. Cheers *PETA - Promoting Enums Through Awareness (www.PE.TA, not to be confused with People Eating Tasty Animals: PETA.org) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [i965][V2] i965/draw: Move constant formation outside of for loop and use an enum.
Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_draw.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c index 6170d07..1b5ed55 100644 --- a/src/mesa/drivers/dri/i965/brw_draw.c +++ b/src/mesa/drivers/dri/i965/brw_draw.c @@ -367,6 +367,12 @@ static bool brw_try_draw_prims( struct gl_context *ctx, bool retval = true; GLuint i; bool fail_next = false; + const int estimated_max_prim_size = + 512 + /* batchbuffer commands */ + ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color + + 1024 + /* gen6 VS push constants */ + 1024 + /* gen6 WM push constants */ + 512; /* misc. pad */ if (ctx-NewState) _mesa_update_state( ctx ); @@ -405,16 +411,6 @@ static bool brw_try_draw_prims( struct gl_context *ctx, brw-state.dirty.brw |= BRW_NEW_VERTICES; for (i = 0; i nr_prims; i++) { - int estimated_max_prim_size; - - estimated_max_prim_size = 512; /* batchbuffer commands */ - estimated_max_prim_size += (BRW_MAX_TEX_UNIT * - (sizeof(struct brw_sampler_state) + - sizeof(struct gen5_sampler_default_color))); - estimated_max_prim_size += 1024; /* gen6 VS push constants */ - estimated_max_prim_size += 1024; /* gen6 WM push constants */ - estimated_max_prim_size += 512; /* misc. pad */ - /* Flush the batch if it's approaching full, so that we don't wrap while * we've got validated state that needs to be in the same batch as the * primitives. -- 1.8.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [i965][V2] i965/draw: Move constant formation outside of for loop and use an enum.
On Thu, Aug 8, 2013 at 2:19 PM, Eric Anholt e...@anholt.net wrote: Mark Mueller markkmuel...@gmail.com writes: Signed-off-by: Mark Mueller markkmuel...@gmail.com --- src/mesa/drivers/dri/i965/brw_draw.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c index 6170d07..1b5ed55 100644 --- a/src/mesa/drivers/dri/i965/brw_draw.c +++ b/src/mesa/drivers/dri/i965/brw_draw.c @@ -367,6 +367,12 @@ static bool brw_try_draw_prims( struct gl_context *ctx, bool retval = true; GLuint i; bool fail_next = false; + const int estimated_max_prim_size = + 512 + /* batchbuffer commands */ + ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color + + 1024 + /* gen6 VS push constants */ + 1024 + /* gen6 WM push constants */ + 512; /* misc. pad */ What's the point of this change? Moving loop invariants out of loops is something basic that your compiler does, Is that universally true for the code as it looked originally (see below)? I've worked on embedded Atom and other systems with heavily dumbed down gcc or other cross compilers. For instance there is a good chance that the compilers from vehicle infotainment systems that I've worked on recently would generate assembly for each line of code below inside the loop. original code: int estimated_max_prim_size; estimated_max_prim_size = 512; /* batchbuffer commands */ estimated_max_prim_size += (BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color))); estimated_max_prim_size += 1024; /* gen6 VS push constants */ estimated_max_prim_size += 1024; /* gen6 WM push constants */ estimated_max_prim_size += 512; /* misc. pad */ . . and you're moving the definition farther from its use. Sure, yet it's in the company of fail_next with a similar problem. What about keeping the definition inside the for loop but adding the const keyword and adding all of the immediates as one operation? for (i = 0; i nr_prims; i++) { const int estimated_max_prim_size = 512 + /* batchbuffer commands */ ((BRW_MAX_TEX_UNIT * (sizeof(struct brw_sampler_state) + sizeof(struct gen5_sampler_default_color + 1024 + /* gen6 VS push constants */ 1024 + /* gen6 WM push constants */ 512; /* misc. pad */ I'm also fine with dropping the whole thing and moving on as the real point of this was to calibrate myself to the contribution process. It's been good for that. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] Removing unnecessary flushes in Gallium
On Thu, Mar 10, 2011 at 12:56 PM, Brian Paul bri...@vmware.com wrote: On 03/10/2011 12:25 PM, Marek Olšák wrote: Hi, I have reviewed where we call flush() and why and some of them seem unnecessary to me. Those flushes may slightly decrease performance, depending on each driver, and may hide driver bugs. glFlush doesn't have to be called in OpenGL so often, and I think state trackers should follow suit. The worst example of this is st/vega. I guess those flushes are there for debugging only. Please review. Marek Olšák (3): st/mesa: remove unnecessary flushes st/vega: remove unnecessary flushes draw: remove unnecessary flush src/gallium/auxiliary/draw/draw_pipe_pstipple.c |7 --- src/gallium/auxiliary/util/u_gen_mipmap.c |2 -- src/gallium/state_trackers/vega/api_images.c|4 src/gallium/state_trackers/vega/image.c |8 src/gallium/state_trackers/vega/mask.c |2 -- src/mesa/state_tracker/st_cb_fbo.c |3 --- 6 files changed, 0 insertions(+), 26 deletions(-) If there aren't any regressions with piglit and the OpenVG demos, looks good. Some time back I did some heavy duty vega work that was focused specifcally on conformance and found the flushes to be important, but that was also on a version of gallium much older than today's code, which I am not as familiar with. Thus YMMV... from my own. One example is that OpenVG uses blend modes that generally aren't supported in most hardware, thus a custom shader is loaded that samples from a texture created immediately prior to the draw via a blt of the actual render target. The flush was important in this case to make sure that the frame buffer was current prior to the creation of that texture copy. My memory is vague but I believe this also applied to mask, filters... probably others in there too. If the render target is safely current without the flush in today's gallium, then the flush probably servers no purpose. Mark ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Merging GLES1/2 to mesa/main
Would it work to put a dummy value into the enum to assure you get the size you want? enum __dri_api_enum { __DRI_API_OPENGL = 0, #define __DRI_API_OPENGL __DRI_API_OPENGL __DRI_API_GLES1 = 1, #define __DRI_API_GLES1 __DRI_API_GLES1 __DRI_API_GLES2 = 2, #define __DRI_API_GLES2 __DRI_API_GLES2 __DRI_API_MAX_VALUE = 0x }; Mark 2010/4/28 Kristian Høgsberg k...@bitplanet.net: 2010/4/28 Jakob Bornecrantz ja...@vmware.com: On 2010-04-28 17.32, Kristian Høgsberg wrote: 2010/4/28 Brian Paulbri...@vmware.com: Kristian Høgsberg wrote: 2010/4/27 Kristian Høgsbergk...@bitplanet.net: [ I hit send to early there... ] review the patches, or at least just some of them. The overall approach is 1. Add a API tag to GLcontext so we key off of that. 2. Use API-aware constructor where we create GLES1/2 contexts (currently only ES1/2 state trackers) 3. Move ES functionality from src/mesa/es into src/mesa/main in a number of steps (this is the bulk of the series). To make this work we have to change some of the tables and generated code so it can all co-exists in the same .so file. 4. Update the DRI interface to allow creating API specific contexts 5. Use the new DRI interface in EGL/DRI2 6. Build libGLES1/2 from the glapi files I'm hoping to merge the branch this week, but if somebody wants to spend a little longer looking over the patches, let me know and I can wait. Looks good, Kristian. Just minor things... 1. Can the __DRI_API_* values be made into a real enum (as you did with gl_api in core Mesa)? And replace the 'int api' parameter with the enum type. I didn't make it an enum because I'm paranoid about using enums in APIs where we're concerned about preserving binary compatibility. The way it's used in the DRI interface, it's only passed to a function and most compilers will probably promote it to an int. Even so, can we keep it as an int in the DRI interface, if I convert it to the mesa enum internally as soon as we get into the DRI driver (that is, in dri2CreateNewContextForAPI)? You want the debuggers to pickup on the value in the backtrace so you don't have to go snooping around with prints to get the textual value. For the paranoid, you can always specify the value for all enums. As well for all the entries define a define for code that expects them to be defines, like so: enum __dri_api_enum { __DRI_API_OPENGL = 0, #define __DRI_API_OPENGL __DRI_API_OPENGL __DRI_API_GLES1 = 1, #define __DRI_API_GLES1 __DRI_API_GLES1 __DRI_API_GLES2 = 2, #define __DRI_API_GLES2 __DRI_API_GLES2 }; It's not about the values that get assigned to the enum members, it's about the size of the enum. The only requirement on the size is that it's big enough to hold the members of the enum, so sizeof(__dri_api_enum) could be 1, 2 or 4, depending on your compiler. You don't want that in your ABI. Kristian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev