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
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.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] [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
---
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-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
---
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, ¶ms);
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,
[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 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
> ---
> 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, ¶ms);
>
> 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_b
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 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
> wrote:
> > fixed subject line
> >
> >
> > On Thu, Nov 14, 2013 at 4:15 PM, Mark Mueller
> > 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
> >> ---
> >> 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,
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 wrote:
> 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
>> ---
>> 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_SI
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 wrote: > On Fri, Nov 15, 2013 at 9:58 PM, Mark Mueller > wrote: > > > > > > > > On Fri, Nov 15, 2013 at 9:52 AM, Marek Olšák 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 wrote: > On Fri, Nov 22, 2013 at 10:45 PM, Mark Mueller > wrote: > > > > > > > > On Fri, Nov 22, 2013 at 12:36 PM, Marek Olšák wrote: > >> > >> On Fri, Nov 15, 2013 at 9:58 PM, Mark Mueller > >> wrote: > >> > > >> > > >> > > >> > On Fri, Nov 15, 2013 at 9:52 AM, Marek Olšák > 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 Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák 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 > 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] [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 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
---
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 wrote:
> Mark Mueller writes:
> > Signed-off-by: Mark Mueller
> > ---
> > 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] 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 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
>
>
> ---
>> 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-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
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 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
> ---
> 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,
> -
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 wrote: > On 12 September 2013 22:06, Chia-I Wu wrote: > >> From: Chia-I Wu >> >> 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 si
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 wrote: > On Mon, Sep 16, 2013 at 4:12 PM, Chia-I Wu wrote: > > On Mon, Sep 16, 2013 at 3:50 AM, Mark Mueller > wrote: > >> > >> > >> > >> On Fri, Sep 13, 2013 at 2:15 PM, Paul Berry > wrote: > >>> > >>> On 12 September 2013 22:06, Chia-I Wu wrote: > >>>> > >>>> From: Chia-I Wu > > >> > >> > >> 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
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 wrote: > 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 06/11] mesa: Add MESA_FORMAT_ABGR2101010.
On Mon, Dec 9, 2013 at 3:09 PM, Paul Berry wrote:
> On 7 December 2013 07:42, Francisco Jerez wrote:
>
>> Paul Berry writes:
>>
>> > On 24 November 2013 21:00, Francisco Jerez
>> 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
>
>
>>
>> > 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 4:10 PM, Marek Olšák wrote: > On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller > wrote: > > > > > > > > On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák 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 wrote: > > > > On Sat, Nov 23, 2013 at 4:10 PM, Marek Olšák wrote: > >> On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller >> wrote: >> > >> > >> > >> > On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák 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 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 wrote: > On 12/17/2013 07:50 PM, Mark Mueller wrote: > >> >> >> >> On Tue, Dec 17, 2013 at 12:19 PM, Mark Mueller > <mailto:markkmuel...@gmail.com>> wrote: >> >> >> >> >> On Sat, Nov 23, 2013 at 4:10 PM, Marek Olšák > <mailto:mar...@gmail.com>> wrote: >> >> On Sat, Nov 23, 2013 at 10:23 PM, Mark Mueller >> mailto:markkmuel...@gmail.com>> wrote: >> > >> > >> > >> > On Sat, Nov 23, 2013 at 2:26 AM, Marek Olšák >> 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 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/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 wrote: > 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 > --- > 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 +
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 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 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
---
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 },
+ [ MES
[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
---
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),
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 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 wrote: > > On Son, 2013-12-22 at 03:46 +0100, Marek Olšák wrote: > >> From: Marek Olšák > >> > >> 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, >
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 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] 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 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 > > 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 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 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
>
> I assume 'make check' actually passes? :)
>
> With the change below,
>
> Reviewed-by: Ian Romanick .
>
> > ---
> > 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 @@
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > + exec="dynamic">
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > + exec="dynamic">
> > +
> > +
> > +
> > +
> > +
> > +
> > + exec="dynamic">
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > 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 @@
> >
> >
> >
> > +http://www.w3.org/2001/XInclude"/>
> > +
> >
> >
> >
> > @@ -8470,7 +8472,7 @@
> >
> > http://www.w3.org/2001/XInclude"/>
> >
> > -
> > +
> >
> > 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 function gl_core_functions_possible[] =
> {
> > { "glInvalidateBufferData", 43, -1 },
> > { "glInvalidateFramebuffer", 43, -1 },
> > { "glInvalidateSubFramebuffer", 43, -1 },
> > -// { "glMultiDrawArraysIndirect", 43, -1 }, // XXX: Add to
> xml
> > -// { "glMultiDrawElementsIndirect", 43, -1 }, // XXX: Add to
> xml
> > + { "glMultiDrawArraysIndirect", 43, -1 },
> > + { "glMultiDrawElementsIndirect", 43, -1 },
> > // { "glGetProgramInterfaceiv", 43, -1 }, // XXX: Add to
> xml
> > // { "glGetProgramResourceIndex", 43, -1 }, // XXX: Add to
> xml
> > // { "glGetProgramResourceName", 43, -1 }, // XXX: Add to
> xml
> > diff --git a/src/mesa/vbo/vbo_exec_array.c
> b/src/mesa/vbo/vbo_exec_array.c
> > index a2c0c7d..4ea10ee 100644
> > --- a/src/mesa/vbo/vbo_exec_array.c
> > +++ b/src/mesa/vbo/vbo_exec_array.c
> > @@ -1564,6 +1564,34 @@
> vbo_exec_DrawTransformFeedbackStreamInstanced(GLenum mode, GLuint name,
> > vbo_draw_transform_feedback(ctx, mode, obj, stream, primcount);
> > }
> >
> > +/**
> > + * Like [Multi]DrawArrays/Elements, but they take most arguments from
> > + * a buffer object.
> > + */
> > +static void
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 wrote: > > > > On Thu, Dec 26, 2013 at 6:57 PM, Michel Dänzer 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 >> > 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
[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 --- 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_FORMAT_XBGR32323232_FLOAT] = N
[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_INTES
[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 --- 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
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 wrote: > On Fri, Jan 17, 2014 at 9:22 AM, Christian König > 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 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 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 > wrote: > > > > > > > > On Fri, Jan 17, 2014 at 8:58 AM, Brian Paul 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
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 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] [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 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 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 >> --- >> > > 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 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 ". 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
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 wrote: > On Thu, Jan 23, 2014 at 4:17 AM, Mark Mueller > wrote: > > Hi Merek, > > I'm Marek. > > > > > > > > > > > > > On Wed, Jan 22, 2014 at 2:49 PM, Marek Olšák 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
[Mesa-dev] [PATCH] mesa: Add warning to _REV pack/unpack functions with incorrect behavior
Signed-off-by: Mark Mueller
---
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 wrote: > On Thu, Jan 23, 2014 at 8:36 PM, Kenneth Graunke > 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 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 > wrote: > > On Sun, Jan 26, 2014 at 11:45 AM, Mark Mueller > > wrote: > >> > >> > >> > >> > >> On Fri, Jan 24, 2014 at 2:50 AM, Marek Olšák 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] [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 wrote: > Eric Anholt 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 > > -- > 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
Re: [Mesa-dev] [PATCH 0/3] Removing unnecessary flushes in Gallium
On Thu, Mar 10, 2011 at 12:56 PM, Brian Paul 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 :
> 2010/4/28 Jakob Bornecrantz :
>> On 2010-04-28 17.32, Kristian Høgsberg wrote:
>>>
>>> 2010/4/28 Brian Paul:
Kristian Høgsberg wrote:
>
> 2010/4/27 Kristian Høgsberg:
> [ 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
