[Mesa-dev] [PATCH 10/10] i965/vs: Return a dummy value when visiting ir_texture.
While the program won't successfully link in the end, this avoids possible assertion failure in the driver during linking if this->result isn't initialized with something already. --- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp index 833349a..61c226a 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp @@ -1675,6 +1675,7 @@ vec4_visitor::visit(ir_texture *ir) * programs that do vertex texturing, but after our visitor has * run. */ + this->result = src_reg(this, glsl_type::vec4_type); } void -- 1.7.5.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/10] mesa: Add a context flag indicating whether two-sided lighting should happen.
The 965 driver was ignoring the VERTEX_PROGRAM_TWO_SIDE flag and only
looking at fixed-function state.
---
src/mesa/main/mtypes.h |2 ++
src/mesa/main/state.c | 18 +-
2 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ae500b4..bcaa2d2 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1979,6 +1979,8 @@ struct gl_vertex_program_state
GLboolean _Enabled; /**< Enabled and _valid_ user program? */
GLboolean PointSizeEnabled; /**< GL_VERTEX_PROGRAM_POINT_SIZE_ARB/NV */
GLboolean TwoSideEnabled; /**< GL_VERTEX_PROGRAM_TWO_SIDE_ARB/NV */
+ /** Computed two sided lighting for fixed function/programs. */
+ GLboolean _TwoSideEnabled;
struct gl_vertex_program *Current; /**< User-bound vertex program */
/** Currently enabled and valid vertex program (including internal
diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
index 457a730..e394e46 100644
--- a/src/mesa/main/state.c
+++ b/src/mesa/main/state.c
@@ -460,7 +460,20 @@ update_clamp_read_color(struct gl_context *ctx)
ctx->Color._ClampReadColor = ctx->Color.ClampReadColor;
}
-
+/**
+ * Update the ctx->VertexProgram._TwoSideEnabled flag.
+ */
+static void
+update_twoside(struct gl_context *ctx)
+{
+ if (ctx->Shader.CurrentVertexProgram ||
+ ctx->VertexProgram.Current) {
+ ctx->VertexProgram._TwoSideEnabled = ctx->VertexProgram.TwoSideEnabled;
+ } else {
+ ctx->VertexProgram._TwoSideEnabled = (ctx->Light.Enabled &&
+ ctx->Light.Model.TwoSide);
+ }
+}
/*
@@ -616,6 +629,9 @@ _mesa_update_state_locked( struct gl_context *ctx )
if (new_state & _NEW_LIGHT)
_mesa_update_lighting( ctx );
+ if (new_state & (_NEW_LIGHT | _NEW_PROGRAM))
+ update_twoside( ctx );
+
if (new_state & (_NEW_LIGHT | _NEW_BUFFERS))
update_clamp_vertex_color(ctx);
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/10] i965: When only BFC is written, use BFC as the color.
Fixes piglit vertex-program-two-side back.
---
src/mesa/drivers/dri/i965/gen6_sf_state.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index bb8bc83..4482e9c 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -67,6 +67,15 @@ get_attr_override(struct brw_vue_map *vue_map, int
urb_entry_read_offset,
/* Find the VUE slot for this attribute. */
slot = vue_map->vert_result_to_slot[vs_attr];
+
+ /* If there was only a back color written but not front, use back
+* as the color instead of undefined
+*/
+ if (slot == -1 && vs_attr == VERT_RESULT_COL0)
+ slot = vue_map->vert_result_to_slot[VERT_RESULT_BFC0];
+ if (slot == -1 && vs_attr == VERT_RESULT_COL1)
+ slot = vue_map->vert_result_to_slot[VERT_RESULT_BFC1];
+
if (slot == -1) {
/* This attribute does not exist in the VUE--that means that the vertex
* shader did not write to it. Behavior is undefined in this case, so
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/10] i965: Respect the VERTEX_PROGRAM_TWO_SIDE flag for shaders.
Fixes piglit:
vertex-program-two-side
vertex-program-two-side primary
vertex-program-two-side secondary
---
src/mesa/drivers/dri/i965/brw_vs_constval.c |6 +++---
src/mesa/drivers/dri/i965/gen6_sf_state.c |5 +++--
src/mesa/drivers/dri/i965/gen7_sf_state.c |6 ++
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vs_constval.c
b/src/mesa/drivers/dri/i965/brw_vs_constval.c
index 67af23e..4d1c4e0 100644
--- a/src/mesa/drivers/dri/i965/brw_vs_constval.c
+++ b/src/mesa/drivers/dri/i965/brw_vs_constval.c
@@ -197,8 +197,8 @@ static void calc_wm_input_sizes( struct brw_context *brw )
memset(&t, 0, sizeof(t));
- /* _NEW_LIGHT */
- if (ctx->Light.Model.TwoSide)
+ /* _NEW_LIGHT | _NEW_PROGRAM */
+ if (ctx->VertexProgram._TwoSideEnabled)
t.twoside = 1;
for (i = 0; i < VERT_ATTRIB_MAX; i++)
@@ -233,7 +233,7 @@ static void calc_wm_input_sizes( struct brw_context *brw )
const struct brw_tracked_state brw_wm_input_sizes = {
.dirty = {
- .mesa = _NEW_LIGHT,
+ .mesa = _NEW_LIGHT | _NEW_PROGRAM,
.brw = BRW_NEW_VERTEX_PROGRAM | BRW_NEW_INPUT_DIMENSIONS,
.cache = 0
},
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 4a9c094..bb8bc83 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -115,7 +115,6 @@ upload_sf_state(struct brw_context *brw)
GLboolean render_to_fbo = brw->intel.ctx.DrawBuffer->Name != 0;
int attr = 0, input_index = 0;
int urb_entry_read_offset;
- int two_side_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide);
float point_size;
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
int nr_userclip;
@@ -285,9 +284,10 @@ upload_sf_state(struct brw_context *brw)
*/
assert(input_index < 16 || attr == input_index);
+ /* _NEW_LIGHT | _NEW_PROGRAM */
attr_overrides[input_index++] =
get_attr_override(&vue_map, urb_entry_read_offset, attr,
- two_side_color);
+ ctx->VertexProgram._TwoSideEnabled);
}
for (; input_index < FRAG_ATTRIB_MAX; input_index++)
@@ -315,6 +315,7 @@ upload_sf_state(struct brw_context *brw)
const struct brw_tracked_state gen6_sf_state = {
.dirty = {
.mesa = (_NEW_LIGHT |
+ _NEW_PROGRAM |
_NEW_POLYGON |
_NEW_LINE |
_NEW_SCISSOR |
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index af98041..85d2d87 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -45,9 +45,6 @@ upload_sbe_state(struct brw_context *brw)
/* _NEW_TRANSFORM */
int urb_entry_read_offset = ctx->Transform.ClipPlanesEnabled ? 2 : 1;
int nr_userclip = brw_count_bits(ctx->Transform.ClipPlanesEnabled);
-
- /* _NEW_LIGHT */
- int two_side_color = (ctx->Light.Enabled && ctx->Light.Model.TwoSide);
uint16_t attr_overrides[FRAG_ATTRIB_MAX];
brw_compute_vue_map(&vue_map, intel, nr_userclip, vs_outputs_written);
@@ -104,7 +101,7 @@ upload_sbe_state(struct brw_context *brw)
attr_overrides[input_index++] =
get_attr_override(&vue_map, urb_entry_read_offset, attr,
- two_side_color);
+ ctx->VertexProgram._TwoSideEnabled);
}
for (; attr < FRAG_ATTRIB_MAX; attr++)
@@ -276,6 +273,7 @@ upload_sf_state(struct brw_context *brw)
const struct brw_tracked_state gen7_sf_state = {
.dirty = {
.mesa = (_NEW_LIGHT |
+ _NEW_PROGRAM |
_NEW_POLYGON |
_NEW_LINE |
_NEW_SCISSOR |
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/10] i965/vs: Add support for compute-to-MRF.
Removes 1.8% of the instructions from 97% of the vertex shaders in
shader-db.
---
src/mesa/drivers/dri/i965/brw_vec4.cpp | 177 +++
src/mesa/drivers/dri/i965/brw_vec4.h|1 +
src/mesa/drivers/dri/i965/brw_vec4_emit.cpp |1 +
3 files changed, 179 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 2a8d63a..4a191af 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -519,4 +519,181 @@ vec4_visitor::move_push_constants_to_pull_constants()
pack_uniform_registers();
}
+/*
+ * Tries to reduce extra MOV instructions by taking GRFs that get just
+ * written and then MOVed into an MRF and making the original write of
+ * the GRF write directly to the MRF instead.
+ */
+bool
+vec4_visitor::opt_compute_to_mrf()
+{
+ bool progress = false;
+ int next_ip = 0;
+
+ calculate_live_intervals();
+
+ foreach_list_safe(node, &this->instructions) {
+ vec4_instruction *inst = (vec4_instruction *)node;
+
+ int ip = next_ip;
+ next_ip++;
+
+ if (inst->opcode != BRW_OPCODE_MOV ||
+ inst->predicate ||
+ inst->dst.file != MRF || inst->src[0].file != GRF ||
+ inst->dst.type != inst->src[0].type ||
+ inst->src[0].abs || inst->src[0].negate || inst->src[0].reladdr)
+continue;
+
+ int mrf = inst->dst.reg;
+
+ /* Can't compute-to-MRF this GRF if someone else was going to
+ * read it later.
+ */
+ if (this->virtual_grf_use[inst->src[0].reg] > ip)
+continue;
+
+ /* We need to check interference with the MRF between this
+ * instruction and the earliest instruction involved in writing
+ * the GRF we're eliminating. To do that, keep track of which
+ * of our source channels we've seen initialized.
+ */
+ bool chans_needed[4] = {false, false, false, false};
+ int chans_remaining = 0;
+ for (int i = 0; i < 4; i++) {
+int chan = BRW_GET_SWZ(inst->src[0].swizzle, i);
+
+if (!(inst->dst.writemask & (1 << i)))
+ continue;
+
+/* We don't handle compute-to-MRF across a swizzle. We would
+ * need to be able to rewrite instructions above to output
+ * results to different channels.
+ */
+if (chan != i)
+ chans_remaining = 5;
+
+if (!chans_needed[chan]) {
+ chans_needed[chan] = true;
+ chans_remaining++;
+}
+ }
+ if (chans_remaining > 4)
+continue;
+
+ /* Now walk up the instruction stream trying to see if we can
+ * rewrite everything writing to the GRF into the MRF instead.
+ */
+ vec4_instruction *scan_inst;
+ for (scan_inst = (vec4_instruction *)inst->prev;
+ scan_inst->prev != NULL;
+ scan_inst = (vec4_instruction *)scan_inst->prev) {
+if (scan_inst->dst.file == GRF &&
+scan_inst->dst.reg == inst->src[0].reg &&
+scan_inst->dst.reg_offset == inst->src[0].reg_offset) {
+ /* Found something writing to the reg we want to turn into
+* a compute-to-MRF.
+*/
+
+ /* SEND instructions can't have MRF as a destination. */
+ if (scan_inst->mlen)
+ break;
+
+ if (intel->gen >= 6) {
+ /* gen6 math instructions must have the destination be
+ * GRF, so no compute-to-MRF for them.
+ */
+ if (scan_inst->is_math()) {
+ break;
+ }
+ }
+
+ /* Mark which channels we found unconditional writes for. */
+ if (!scan_inst->predicate) {
+ for (int i = 0; i < 4; i++) {
+ if (scan_inst->dst.writemask & (1 << i) &&
+ chans_needed[i]) {
+chans_needed[i] = false;
+chans_remaining--;
+ }
+ }
+ }
+
+ if (chans_remaining == 0)
+ break;
+}
+
+/* We don't handle flow control here. Most computation of
+ * values that end up in MRFs are shortly before the MRF
+ * write anyway.
+ */
+if (scan_inst->opcode == BRW_OPCODE_DO ||
+scan_inst->opcode == BRW_OPCODE_WHILE ||
+scan_inst->opcode == BRW_OPCODE_ELSE ||
+scan_inst->opcode == BRW_OPCODE_ENDIF) {
+ break;
+}
+
+/* You can't read from an MRF, so if someone else reads our
+ * MRF's source GRF that we wanted to rewrite, that stops us.
+ */
+bool interfered = false;
+for (int i = 0; i < 3; i++) {
+ if (scan_inst->src[i].file == GRF &&
+ scan_inst->src[i].reg == inst->src[0].reg &&
+ scan_inst->src[i].reg_offset == inst->src[0].reg_offset) {
+ interfered = true;
+ }
+}
+
[Mesa-dev] [PATCH 05/10] i965/vs: Do VUE writes using the MRF file instead of hardware register.
We'll only do compute-to-MRF on accesses to this file.
---
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 83f543f..833349a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -1831,13 +1831,15 @@ vec4_visitor::emit_clip_distances(struct brw_reg reg,
int offset)
void
vec4_visitor::emit_urb_slot(int mrf, int vert_result)
{
- struct brw_reg reg = brw_message_reg(mrf);
+ struct brw_reg hw_reg = brw_message_reg(mrf);
+ dst_reg reg = dst_reg(MRF, mrf);
+ reg.type = BRW_REGISTER_TYPE_F;
switch (vert_result) {
case VERT_RESULT_PSIZ:
/* PSIZ is always in slot 0, and is coupled with other flags. */
current_annotation = "indices, point width, clip flags";
- emit_psiz_and_flags(reg);
+ emit_psiz_and_flags(hw_reg);
break;
case BRW_VERT_RESULT_NDC:
current_annotation = "NDC";
@@ -1850,11 +1852,11 @@ vec4_visitor::emit_urb_slot(int mrf, int vert_result)
break;
case BRW_VERT_RESULT_CLIP0:
current_annotation = "user clip distances";
- emit_clip_distances(reg, 0);
+ emit_clip_distances(hw_reg, 0);
break;
case BRW_VERT_RESULT_CLIP1:
current_annotation = "user clip distances";
- emit_clip_distances(reg, 4);
+ emit_clip_distances(hw_reg, 4);
break;
case BRW_VERT_RESULT_PAD:
/* No need to write to this slot */
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/10] i965/vs: Add a function for how many MRFs get written as part of a SEND.
This will be used for compute-to-mrf, which needs to know when MRFs
get overwritten.
---
src/mesa/drivers/dri/i965/brw_vec4.cpp | 37
src/mesa/drivers/dri/i965/brw_vec4.h |2 +
2 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 5fd4756..2a8d63a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -43,6 +43,43 @@ vec4_instruction::is_math()
opcode == SHADER_OPCODE_COS ||
opcode == SHADER_OPCODE_POW);
}
+/**
+ * Returns how many MRFs an opcode will write over.
+ *
+ * Note that this is not the 0 or 1 implied writes in an actual gen
+ * instruction -- the generate_* functions generate additional MOVs
+ * for setup.
+ */
+int
+vec4_visitor::implied_mrf_writes(vec4_instruction *inst)
+{
+ if (inst->mlen == 0)
+ return 0;
+
+ switch (inst->opcode) {
+ case SHADER_OPCODE_RCP:
+ case SHADER_OPCODE_RSQ:
+ case SHADER_OPCODE_SQRT:
+ case SHADER_OPCODE_EXP2:
+ case SHADER_OPCODE_LOG2:
+ case SHADER_OPCODE_SIN:
+ case SHADER_OPCODE_COS:
+ return 1;
+ case SHADER_OPCODE_POW:
+ return 2;
+ case VS_OPCODE_URB_WRITE:
+ return 1;
+ case VS_OPCODE_PULL_CONSTANT_LOAD:
+ return 2;
+ case VS_OPCODE_SCRATCH_READ:
+ return 2;
+ case VS_OPCODE_SCRATCH_WRITE:
+ return 3;
+ default:
+ assert(!"not reached");
+ return inst->mlen;
+ }
+}
bool
src_reg::equals(src_reg *r)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
b/src/mesa/drivers/dri/i965/brw_vec4.h
index 0f85cdb..e305e6f 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -439,6 +439,8 @@ public:
vec4_instruction *SCRATCH_READ(dst_reg dst, src_reg index);
vec4_instruction *SCRATCH_WRITE(dst_reg dst, src_reg src, src_reg index);
+ int implied_mrf_writes(vec4_instruction *inst);
+
bool try_rewrite_rhs_to_dst(ir_assignment *ir,
dst_reg dst,
src_reg src,
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/10] i965/vs: Remove dead fields of src_reg.
These were copy and pasted from the FS, and are never used.
---
src/mesa/drivers/dri/i965/brw_vec4.h |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
b/src/mesa/drivers/dri/i965/brw_vec4.h
index 3f116ee..0f85cdb 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -83,9 +83,7 @@ public:
int reg_offset;
/** Register type. BRW_REGISTER_TYPE_* */
int type;
- bool sechalf;
struct brw_reg fixed_hw_reg;
- int smear; /* -1, or a channel of the reg to smear to all channels. */
/** Value for file == BRW_IMMMEDIATE_FILE */
union {
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/10] i965/vs: Handle destinations in the MRF file.
We've been referencing MRFs through the HW_REG file so far, but that makes it harder to handle compute-to-MRF and similar optimizations. --- src/mesa/drivers/dri/i965/brw_vec4_emit.cpp |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp index 7031d2a..15f2458 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp @@ -165,6 +165,12 @@ vec4_instruction::get_dst(void) brw_reg.dw1.bits.writemask = dst.writemask; break; + case MRF: + brw_reg = brw_message_reg(dst.reg + dst.reg_offset); + brw_reg = retype(brw_reg, dst.type); + brw_reg.dw1.bits.writemask = dst.writemask; + break; + case HW_REG: brw_reg = dst.fixed_hw_reg; break; -- 1.7.5.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/10] i965/vs: Add support for simple algebraic optimizations.
We generate silly code for array access, and it's easier to generally
support the cleanup than to specifically avoid the bad code in each
place we might generate it.
Removes 4.6% of instructions from 41.6% of shaders in shader-db,
particularly savage2/hon and unigine.
---
src/mesa/drivers/dri/i965/brw_vec4.cpp | 91 +++
src/mesa/drivers/dri/i965/brw_vec4.h|1 +
src/mesa/drivers/dri/i965/brw_vec4_emit.cpp |1 +
3 files changed, 93 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 436de2f..5fd4756 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -306,6 +306,97 @@ vec4_visitor::pack_uniform_registers()
}
}
+static bool
+src_reg_is_zero(src_reg *reg)
+{
+ if (reg->file != IMM)
+ return false;
+
+ if (reg->type == BRW_REGISTER_TYPE_F) {
+ return reg->imm.f == 0.0;
+ } else {
+ return reg->imm.i == 0;
+ }
+}
+
+static bool
+src_reg_is_one(src_reg *reg)
+{
+ if (reg->file != IMM)
+ return false;
+
+ if (reg->type == BRW_REGISTER_TYPE_F) {
+ return reg->imm.f == 1.0;
+ } else {
+ return reg->imm.i == 1;
+ }
+}
+
+/**
+ * Does algebraic optimizations (0 * a = 0, 1 * a = a, a + 0 = a).
+ *
+ * While GLSL IR also performs this optimization, we end up with it in
+ * our instruction stream for a couple of reasons. One is that we
+ * sometimes generate silly instructions, for example in array access
+ * where we'll generate "ADD offset, index, base" even if base is 0.
+ * The other is that GLSL IR's constant propagation doesn't track the
+ * components of aggregates, so some VS patterns (initialize matrix to
+ * 0, accumulate in vertex blending factors) end up breaking down to
+ * instructions involving 0.
+ */
+bool
+vec4_visitor::opt_algebraic()
+{
+ bool progress = false;
+
+ foreach_list(node, &this->instructions) {
+ vec4_instruction *inst = (vec4_instruction *)node;
+
+ switch (inst->opcode) {
+ case BRW_OPCODE_ADD:
+if (src_reg_is_zero(&inst->src[1])) {
+ inst->opcode = BRW_OPCODE_MOV;
+ inst->src[1] = src_reg();
+ progress = true;
+}
+break;
+
+ case BRW_OPCODE_MUL:
+if (src_reg_is_zero(&inst->src[1])) {
+ inst->opcode = BRW_OPCODE_MOV;
+ switch (inst->src[0].type) {
+ case BRW_REGISTER_TYPE_F:
+ inst->src[0] = src_reg(0.0f);
+ break;
+ case BRW_REGISTER_TYPE_D:
+ inst->src[0] = src_reg(0);
+ break;
+ case BRW_REGISTER_TYPE_UD:
+ inst->src[0] = src_reg(0u);
+ break;
+ default:
+ assert(!"not reached");
+ inst->src[0] = src_reg(0.0f);
+ break;
+ }
+ inst->src[1] = src_reg();
+ progress = true;
+} else if (src_reg_is_one(&inst->src[1])) {
+ inst->opcode = BRW_OPCODE_MOV;
+ inst->src[1] = src_reg();
+}
+break;
+ default:
+break;
+ }
+ }
+
+ if (progress)
+ this->live_intervals_valid = false;
+
+ return progress;
+}
+
/**
* Only a limited number of hardware registers may be used for push
* constants, so this turns access to the overflowed constants into
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
b/src/mesa/drivers/dri/i965/brw_vec4.h
index 7739a15..3f116ee 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -401,6 +401,7 @@ public:
bool dead_code_eliminate();
bool virtual_grf_interferes(int a, int b);
bool opt_copy_propagation();
+ bool opt_algebraic();
vec4_instruction *emit(vec4_instruction *inst);
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
index c40c41f..7031d2a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
@@ -615,6 +615,7 @@ vec4_visitor::run()
progress = false;
progress = dead_code_eliminate() || progress;
progress = opt_copy_propagation() || progress;
+ progress = opt_algebraic() || progress;
} while (progress);
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] two-side fixes, i965 new-VS optimization
For those that don't care for i965, there's patch 7/10 to look at, to go along with the proposed vertex-program-two-side piglit test. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] mesa: Add support for Begin/EndConditionalRender in display lists.
Fixes piglit nv_conditional_render-dlist.
---
src/mesa/main/dlist.c | 45 +
1 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index 6e075b4..a80cfdc 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -459,6 +459,10 @@ typedef enum
/* GL_ARB_sync */
OPCODE_WAIT_SYNC,
+ /* GL_NV_conditional_render */
+ OPCODE_BEGIN_CONDITIONAL_RENDER,
+ OPCODE_END_CONDITIONAL_RENDER,
+
/* The following three are meta instructions */
OPCODE_ERROR,/* raise compiled-in error */
OPCODE_CONTINUE,
@@ -7371,6 +7375,35 @@ save_WaitSync(GLsync sync, GLbitfield flags, GLuint64
timeout)
}
+/** GL_NV_conditional_render */
+static void GLAPIENTRY
+save_BeginConditionalRender(GLuint queryId, GLenum mode)
+{
+ GET_CURRENT_CONTEXT(ctx);
+ Node *n;
+ ASSERT_OUTSIDE_SAVE_BEGIN_END_AND_FLUSH(ctx);
+ n = alloc_instruction(ctx, OPCODE_BEGIN_CONDITIONAL_RENDER, 2);
+ if (n) {
+ n[1].i = queryId;
+ n[2].e = mode;
+ }
+ if (ctx->ExecuteFlag) {
+ CALL_BeginConditionalRenderNV(ctx->Exec, (queryId, mode));
+ }
+}
+
+static void GLAPIENTRY
+save_EndConditionalRender()
+{
+ GET_CURRENT_CONTEXT(ctx);
+ ASSERT_OUTSIDE_SAVE_BEGIN_END_AND_FLUSH(ctx);
+ alloc_instruction(ctx, OPCODE_END_CONDITIONAL_RENDER, 0);
+ if (ctx->ExecuteFlag) {
+ CALL_EndConditionalRenderNV(ctx->Exec, ());
+ }
+}
+
+
/**
* Save an error-generating command into display list.
*
@@ -8636,6 +8669,14 @@ execute_list(struct gl_context *ctx, GLuint list)
}
break;
+ /* GL_NV_conditional_render */
+ case OPCODE_BEGIN_CONDITIONAL_RENDER:
+CALL_BeginConditionalRenderNV(ctx->Exec, (n[1].i, n[2].e));
+break;
+ case OPCODE_END_CONDITIONAL_RENDER:
+CALL_EndConditionalRenderNV(ctx->Exec, ());
+break;
+
case OPCODE_CONTINUE:
n = (Node *) n[1].next;
break;
@@ -10340,6 +10381,10 @@ _mesa_create_save_table(void)
SET_FramebufferTextureARB(table, save_FramebufferTexture);
SET_FramebufferTextureFaceARB(table, save_FramebufferTextureFace);
+ /* GL_NV_conditional_render */
+ SET_BeginConditionalRenderNV(table, save_BeginConditionalRender);
+ SET_EndConditionalRenderNV(table, save_EndConditionalRender);
+
/* GL_ARB_sync */
_mesa_init_sync_dispatch(table);
SET_WaitSync(table, save_WaitSync);
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] mesa: Throw an error instead of asserting for condrender with query == 0.
>From the NV_conditional_render spec:
BeginQuery sets the active query object name for the query type given by
to . If BeginQuery is called with an of zero, if the
active query object name for is non-zero, if is the active
query object name for any query type, or if is the active query
object for condtional rendering (Section 2.X), the error INVALID OPERATION
is generated.
Fixes piglit nv_conditional_render-begin-zero.
---
src/mesa/main/condrender.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/mesa/main/condrender.c b/src/mesa/main/condrender.c
index 352e2e2..c8195a5 100644
--- a/src/mesa/main/condrender.c
+++ b/src/mesa/main/condrender.c
@@ -44,7 +44,8 @@ _mesa_BeginConditionalRender(GLuint queryId, GLenum mode)
struct gl_query_object *q;
GET_CURRENT_CONTEXT(ctx);
- if (!ctx->Extensions.NV_conditional_render || ctx->Query.CondRenderQuery) {
+ if (!ctx->Extensions.NV_conditional_render || ctx->Query.CondRenderQuery ||
+ queryId == 0) {
_mesa_error(ctx, GL_INVALID_OPERATION, "glBeginConditionalRender()");
return;
}
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] mesa: Throw an error when starting conditional render on an active query.
>From the NV_conditional_render spec:
BeginQuery sets the active query object name for the query type given by
to . If BeginQuery is called with an of zero, if the
active query object name for is non-zero, if is the active
query object name for any query type, or if is the active query
object for condtional rendering (Section 2.X), the error INVALID OPERATION
is generated.
Fixes piglit nv_conditional_render-begin-while-active.
---
src/mesa/main/condrender.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/mesa/main/condrender.c b/src/mesa/main/condrender.c
index c8195a5..57f3715 100644
--- a/src/mesa/main/condrender.c
+++ b/src/mesa/main/condrender.c
@@ -73,7 +73,7 @@ _mesa_BeginConditionalRender(GLuint queryId, GLenum mode)
}
ASSERT(q->Id == queryId);
- if (q->Target != GL_SAMPLES_PASSED) {
+ if (q->Target != GL_SAMPLES_PASSED || q->Active) {
_mesa_error(ctx, GL_INVALID_OPERATION, "glBeginConditionalRender()");
return;
}
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] [PATCH 2/2] i965: setup the edge flag enable bit in VE on SNB+
On Thu, Sep 08, 2011 at 09:25:30PM -0700, Kenneth Graunke wrote: > On 09/08/2011 06:59 PM, Yuanhan Liu wrote: > > On Thu, Sep 08, 2011 at 08:39:46AM -0700, Eric Anholt wrote: > >> On Thu, 8 Sep 2011 11:00:52 +0800, Yuanhan Liu > >> wrote: > >>> BTW, this patch fix the oglc pntrast fail on SNB(haven't tested it on > >>> IVB yet). > >> > >> Could you include a piglit test for the failure? > > > > Actually, this is an issue of edgeflag. You will also find this patch > > will fix the oglc edgeflag test fail. pntrast test case would also use > > polygonmode with edgeflag to render a point. Thus it failed. > > > > I simply grep-ed the piglit repo, didn't find any references on > > glEdgeFlag. Seems that current piglit doesn't include this test? > > You are right---piglit doesn't currently test glEdgeFlag. I think Eric > was asking if you could please write a new test for piglit that > demonstrates the failure. The rest of the community doesn't have access > to closed source test suites like oglc, but everyone can run piglit. > Having the test will help other developers avoid accidentally break > glEdgeFlag support in the future. Got it. I may try to write one next week. Besides this, any ideas on how to setup the edge flag enable bit? Or, does that patch make sense to you. If Ok, I will resend the patch later with little change to apply Eric's suggestion. Thanks, Yuanhan Liu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: fix the wrong code to detect null texture.
On Thu, Sep 08, 2011 at 09:12:44PM -0700, Kenneth Graunke wrote:
> On 09/08/2011 07:56 PM, Yuanhan Liu wrote:
> > There is already comments show how to detect a null texture. Fix the
> > code to match the comments.
> >
> > This would fix the oglc divzero(basic.texQOrWEqualsZero) and
> > divzero(basic.texTrivialPrim) test case fail.
> >
> > Signed-off-by: Yuanhan Liu
> > ---
> > src/mesa/drivers/dri/intel/intel_mipmap_tree.c |2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> > index f36240d..717e0ae 100644
> > --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> > @@ -137,7 +137,7 @@ intel_miptree_create(struct intel_context *intel,
> > /*
> > * pitch == 0 || height == 0 indicates the null texture
> > */
> > - if (!mt || !mt->total_height) {
> > + if (!mt->total_width || !mt->total_height) {
> >free(mt);
> >return NULL;
> > }
>
> I was a bit skeptical about removing the !mt check, but I think it's
> actually okay since intel_miptree_create_internal never returns NULL.
>
> I had to convince myself of that by writing the patch I'm about to reply
> with. intel_miptree_create_internal has explicit "if (!ok) return NULL"
> code in it, but it turns out that ok is statically provable to always be
> true. My patch deletes it.
>
> The only other reason it might want to return NULL is if allocation of
> the struct intel_mipmap_tree fails, but it doesn't check for that.
> Perhaps it should? (But, it's a tiny struct and I'm pretty sure we fail
> to check things like that all over the place, so...eh...)
>
> Anyway, this looks right to me. I'll defer to Eric and Ian though.
Thanks. BTW, would any of you push my patches(including those patches I
sent days before) to mesa repo? I haven't had the commit permission yet.
Thanks,
Yuanhan Liu
>
> Reviewed-by: Kenneth Graunke
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #6 from Alexandre Demers 2011-09-08 22:13:10 PDT --- Review of attachment 50992: --> (https://bugs.freedesktop.org/review?bug=40729&attachment=50992) Need to replace "struct pipe_present_control ctrl;" by "struct native_present_control ctrl;" at line 1109. Once done, it fixes the build. Alex -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #5 from Alexandre Demers 2011-09-08 22:10:52 PDT --- OK, I think I got it working. You must have meant to replace "pipe" by "native" at line 1109. Doing this fixed your patch and allowed me to build without error. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: fix the wrong code to detect null texture.
On Fri, Sep 09, 2011 at 10:56:36AM +0800, Yuanhan Liu wrote:
> There is already comments show how to detect a null texture. Fix the
> code to match the comments.
>
> This would fix the oglc divzero(basic.texQOrWEqualsZero) and
> divzero(basic.texTrivialPrim) test case fail.
>
> Signed-off-by: Yuanhan Liu
This patch also fix the two oglc fail on i915gm machine.
Thanks,
Yuanhan Liu
> ---
> src/mesa/drivers/dri/intel/intel_mipmap_tree.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index f36240d..717e0ae 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -137,7 +137,7 @@ intel_miptree_create(struct intel_context *intel,
> /*
> * pitch == 0 || height == 0 indicates the null texture
> */
> - if (!mt || !mt->total_height) {
> + if (!mt->total_width || !mt->total_height) {
>free(mt);
>return NULL;
> }
> --
> 1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #4 from Alexandre Demers 2011-09-08 22:03:17 PDT --- (In reply to comment #3) > (In reply to comment #2) > > src/dxgi_native.cpp: In member function ‘virtual HRESULT > > GalliumDXGISwapChain::Present(UINT, UINT)’: > > src/dxgi_native.cpp:1109:31: error: aggregate > > ‘GalliumDXGISwapChain::Present(UINT, UINT)::pipe_present_control ctrl’ has > > incomplete type and cannot be defined > Does s/pipe/native/ at src/dxgi_native.cpp:1109 help? Sorry, what? I think there is something missing in the action I should perform. ;) -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #3 from Chia-I Wu 2011-09-08 21:56:10 PDT --- (In reply to comment #2) > src/dxgi_native.cpp: In member function ‘virtual HRESULT > GalliumDXGISwapChain::Present(UINT, UINT)’: > src/dxgi_native.cpp:1109:31: error: aggregate > ‘GalliumDXGISwapChain::Present(UINT, UINT)::pipe_present_control ctrl’ has > incomplete type and cannot be defined Does s/pipe/native/ at src/dxgi_native.cpp:1109 help? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #2 from Alexandre Demers 2011-09-08 21:41:56 PDT --- Not quite yet: g++ -c -I. -I../../../../../src/gallium/include -I../../../../../src/gallium/auxiliary -I../../../../../src/gallium/drivers -I../../../../../include -Iinclude -I../gd3dapi -I../d3dapi -I../w32api -I../d3d1xstutil/include -I../include -I../../../include -I../../../auxiliary -I../../../state_trackers/egl/common -g -O2 -Wall -fno-strict-aliasing -m64 -g -fPIC -D_GNU_SOURCE -DPTHREADS -DDEBUG -DTEXTURE_FLOAT_ENABLED -DHAVE_POSIX_MEMALIGN -DMESA_SELINUX -DUSE_XCB -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DGLX_USE_TLS -DPTHREADS -DUSE_EXTERNAL_DXTN_LIB=1 -DIN_DRI_DRIVER -DHAVE_ALIAS -DHAVE_MINCORE -DHAVE_LIBUDEV -DHAVE_XCB_DRI2 -DXCB_DRI2_CONNECT_DEVICE_NAME_BROKEN -DHAVE_XEXTPROTO_71 -D__STDC_CONSTANT_MACROS -DHAVE_LLVM=0x0208 -fvisibility=hidden -DDXGI_DRIVER_SEARCH_DIR=\"/usr/lib/x86_64-linux-gnu/egl\" -I/usr/lib/llvm-2.8/include -D_GNU_SOURCE -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DGALLIUM_DXGI_USE_X11 -DGALLIUM_DXGI_USE_DRM src/dxgi_native.cpp -o src/dxgi_native.o In file included from src/dxgi_private.h:34:0, from src/dxgi_native.cpp:27: ../d3d1xstutil/include/d3d1xstutil.h:250:0: warning: "__uuidof" redefined ../w32api/guiddef.h:50:0: note: this is the location of the previous definition In file included from ../w32api/windef.h:252:0, from ../w32api/windows.h:41, from ../w32api/rpc.h:29, from ../w32api/objbase.h:19, from ../d3d1xstutil/include/d3d1xstutil.h:45, from src/dxgi_private.h:34, from src/dxgi_native.cpp:27: ../w32api/winnt.h:685:35: warning: attributes ignored on elaborated-type-specifier that is not a forward declaration In file included from ../w32api/objbase.h:20:0, from ../d3d1xstutil/include/d3d1xstutil.h:45, from src/dxgi_private.h:34, from src/dxgi_native.cpp:27: ../w32api/rpcndr.h:177:1: warning: ‘_MIDL_STUB_MESSAGE’ has a field ‘_MIDL_STUB_MESSAGE::SavedContextHandles’ whose type uses the anonymous namespace ../w32api/rpcndr.h:479:32: warning: ‘_SCONTEXT_QUEUE’ has a field ‘_SCONTEXT_QUEUE::ArrayOfObjects’ whose type uses the anonymous namespace src/dxgi_native.cpp: In member function ‘virtual HRESULT GalliumDXGISwapChain::Present(UINT, UINT)’: src/dxgi_native.cpp:1109:31: error: aggregate ‘GalliumDXGISwapChain::Present(UINT, UINT)::pipe_present_control ctrl’ has incomplete type and cannot be defined make[5]: *** [src/dxgi_native.o] Error 1 -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] [PATCH 2/2] i965: setup the edge flag enable bit in VE on SNB+
On 09/08/2011 06:59 PM, Yuanhan Liu wrote: > On Thu, Sep 08, 2011 at 08:39:46AM -0700, Eric Anholt wrote: >> On Thu, 8 Sep 2011 11:00:52 +0800, Yuanhan Liu >> wrote: >>> BTW, this patch fix the oglc pntrast fail on SNB(haven't tested it on >>> IVB yet). >> >> Could you include a piglit test for the failure? > > Actually, this is an issue of edgeflag. You will also find this patch > will fix the oglc edgeflag test fail. pntrast test case would also use > polygonmode with edgeflag to render a point. Thus it failed. > > I simply grep-ed the piglit repo, didn't find any references on > glEdgeFlag. Seems that current piglit doesn't include this test? You are right---piglit doesn't currently test glEdgeFlag. I think Eric was asking if you could please write a new test for piglit that demonstrates the failure. The rest of the community doesn't have access to closed source test suites like oglc, but everyone can run piglit. Having the test will help other developers avoid accidentally break glEdgeFlag support in the future. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: Remove pointless boolean return value from *_miptree_layout.
i915_miptree_layout, i945_miptree_layout, and brw_miptree_layout always
just return GL_TRUE, so there's really no point to it. Change them to
void functions and remove the (dead) error checking code.
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i915/i915_tex_layout.c|8 ++--
src/mesa/drivers/dri/i965/brw_tex_layout.c |9 -
src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 13 +++--
src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 18 +-
4 files changed, 18 insertions(+), 30 deletions(-)
Compile tested only.
diff --git a/src/mesa/drivers/dri/i915/i915_tex_layout.c
b/src/mesa/drivers/dri/i915/i915_tex_layout.c
index e6a4711..c1450be 100644
--- a/src/mesa/drivers/dri/i915/i915_tex_layout.c
+++ b/src/mesa/drivers/dri/i915/i915_tex_layout.c
@@ -230,7 +230,7 @@ i915_miptree_layout_2d(struct intel_context *intel,
}
}
-GLboolean
+void
i915_miptree_layout(struct intel_context *intel, struct intel_mipmap_tree * mt,
uint32_t tiling)
{
@@ -253,8 +253,6 @@ i915_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree * mt,
DBG("%s: %dx%dx%d\n", __FUNCTION__,
mt->total_width, mt->total_height, mt->cpp);
-
- return GL_TRUE;
}
@@ -466,7 +464,7 @@ i945_miptree_layout_3d(struct intel_context *intel,
}
}
-GLboolean
+void
i945_miptree_layout(struct intel_context *intel, struct intel_mipmap_tree * mt,
uint32_t tiling)
{
@@ -492,6 +490,4 @@ i945_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree * mt,
DBG("%s: %dx%dx%d\n", __FUNCTION__,
mt->total_width, mt->total_height, mt->cpp);
-
- return GL_TRUE;
}
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index b5d2cf3..33d8cf0 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -39,9 +39,10 @@
#define FILE_DEBUG_FLAG DEBUG_MIPTREE
-GLboolean brw_miptree_layout(struct intel_context *intel,
-struct intel_mipmap_tree *mt,
-uint32_t tiling)
+void
+brw_miptree_layout(struct intel_context *intel,
+ struct intel_mipmap_tree *mt,
+ uint32_t tiling)
{
/* XXX: these vary depending on image format: */
/* GLint align_w = 4; */
@@ -167,7 +168,5 @@ GLboolean brw_miptree_layout(struct intel_context *intel,
}
DBG("%s: %dx%dx%d\n", __FUNCTION__,
mt->total_width, mt->total_height, mt->cpp);
-
- return GL_TRUE;
}
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index f36240d..9b53fdb 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -64,7 +64,6 @@ intel_miptree_create_internal(struct intel_context *intel,
GLuint depth0,
uint32_t tiling)
{
- GLboolean ok;
struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
int compress_byte = 0;
@@ -89,19 +88,13 @@ intel_miptree_create_internal(struct intel_context *intel,
#ifdef I915
if (intel->is_945)
- ok = i945_miptree_layout(intel, mt, tiling);
+ i945_miptree_layout(intel, mt, tiling);
else
- ok = i915_miptree_layout(intel, mt, tiling);
+ i915_miptree_layout(intel, mt, tiling);
#else
- ok = brw_miptree_layout(intel, mt, tiling);
+ brw_miptree_layout(intel, mt, tiling);
#endif
- if (!ok) {
- free(mt);
- DBG("%s not okay - returning NULL\n", __FUNCTION__);
- return NULL;
- }
-
return mt;
}
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
index ea86590..d0e1c40 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
@@ -212,14 +212,14 @@ void intel_miptree_image_copy(struct intel_context *intel,
/* i915_mipmap_tree.c:
*/
-GLboolean i915_miptree_layout(struct intel_context *intel,
- struct intel_mipmap_tree *mt,
- uint32_t tiling);
-GLboolean i945_miptree_layout(struct intel_context *intel,
- struct intel_mipmap_tree *mt,
- uint32_t tiling);
-GLboolean brw_miptree_layout(struct intel_context *intel,
-struct intel_mipmap_tree *mt,
-uint32_t tiling);
+void i915_miptree_layout(struct intel_context *intel,
+struct intel_mipmap_tree *mt,
+uint32_t tiling);
+void i945_miptree_layout(struct intel_context *intel,
+struct intel_mipmap_tree *mt,
+uint32_t tiling);
+void brw_miptree_layout(struct intel_context *intel,
+ struct intel_mipmap_tree *mt,
+ uint32_t tilin
Re: [Mesa-dev] [PATCH] intel: fix the wrong code to detect null texture.
On 09/08/2011 07:56 PM, Yuanhan Liu wrote:
> There is already comments show how to detect a null texture. Fix the
> code to match the comments.
>
> This would fix the oglc divzero(basic.texQOrWEqualsZero) and
> divzero(basic.texTrivialPrim) test case fail.
>
> Signed-off-by: Yuanhan Liu
> ---
> src/mesa/drivers/dri/intel/intel_mipmap_tree.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> index f36240d..717e0ae 100644
> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
> @@ -137,7 +137,7 @@ intel_miptree_create(struct intel_context *intel,
> /*
> * pitch == 0 || height == 0 indicates the null texture
> */
> - if (!mt || !mt->total_height) {
> + if (!mt->total_width || !mt->total_height) {
>free(mt);
>return NULL;
> }
I was a bit skeptical about removing the !mt check, but I think it's
actually okay since intel_miptree_create_internal never returns NULL.
I had to convince myself of that by writing the patch I'm about to reply
with. intel_miptree_create_internal has explicit "if (!ok) return NULL"
code in it, but it turns out that ok is statically provable to always be
true. My patch deletes it.
The only other reason it might want to return NULL is if allocation of
the struct intel_mipmap_tree fails, but it doesn't check for that.
Perhaps it should? (But, it's a tiny struct and I'm pretty sure we fail
to check things like that all over the place, so...eh...)
Anyway, this looks right to me. I'll defer to Eric and Ian though.
Reviewed-by: Kenneth Graunke
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 --- Comment #1 from Chia-I Wu 2011-09-08 21:01:08 PDT --- Created an attachment (id=50992) View: https://bugs.freedesktop.org/attachment.cgi?id=50992 Review: https://bugs.freedesktop.org/review?bug=40729&attachment=50992 fix the build error Yes. Does this patch help? -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: fix the wrong code to detect null texture.
There is already comments show how to detect a null texture. Fix the
code to match the comments.
This would fix the oglc divzero(basic.texQOrWEqualsZero) and
divzero(basic.texTrivialPrim) test case fail.
Signed-off-by: Yuanhan Liu
---
src/mesa/drivers/dri/intel/intel_mipmap_tree.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index f36240d..717e0ae 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -137,7 +137,7 @@ intel_miptree_create(struct intel_context *intel,
/*
* pitch == 0 || height == 0 indicates the null texture
*/
- if (!mt || !mt->total_height) {
+ if (!mt->total_width || !mt->total_height) {
free(mt);
return NULL;
}
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: fix null 1D texture handling
On Thu, Sep 08, 2011 at 01:16:23PM -0700, Ian Romanick wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 09/08/2011 03:16 AM, Yuanhan Liu wrote:
> > If user call glTexImage1D with width = 0 and height = 0(the last
> > level) like this: glTexImage1D(GL_TEXTURE_1D, level, interfomart,
> > 0, 0, format, type, pixel)
> >
> > It would generate a SIGSEGV fault. As i945_miptree_layout_2d didn't
> > handle this special case. More info are commented in line in this
> > patch.
> >
> > This would fix the oglc divzero(basic.texQOrWEqualsZero) and
> > divzero(basic.texTrivialPrim) test case fail.
> >
> > Signed-off-by: Yuanhan Liu
>
> This feels like a band-aid. Can this problem also occur on pre-i945
> hardware (that use i915_miptree_layout_2d)?
Finally find a 915gm machine. But stuff totally broken on that machine.
That give me no chance to test it. Will test it when broken fixed.
>
> > --- src/mesa/drivers/dri/intel/intel_tex_layout.c | 12
> > 1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c
> > b/src/mesa/drivers/dri/intel/intel_tex_layout.c index
> > 9d81523..4ec71c2 100644 ---
> > a/src/mesa/drivers/dri/intel/intel_tex_layout.c +++
> > b/src/mesa/drivers/dri/intel/intel_tex_layout.c @@ -61,6 +61,18 @@
> > void i945_miptree_layout_2d(struct intel_context *intel, GLuint
> > width = mt->width0; GLuint height = mt->height0;
> >
> > + if (width == 0 && _mesa_get_texture_dimensions(mt->target) ==
> > 1) {
>
> Does the dimensionality of the texture target actually matter? It
In fact, it's not about the texture target, it's about the way how to
detect it is a null texture(width=0, height=0) or not. A user might call
glTexImage1D(1D, ... width=0, ..height=0 ..), but mesa would set the width
to 1 later at _mesa_TexImage1D. That result i945_miptree_layout_2d would
return a mt with total_height = 2 = ALIGN(height = 1, 2).
Here is how the oglc divzero(basic.texQOrWEqualsZero) trigger this
segfalut error:
It calls:
glTexImage1D(GL_TEXTURE_1D, 0, 3, 8, 0, GL_RGB, GL_FLOAT, pixel);
glTexImage1D(GL_TEXTURE_1D, 1, 3, 4, 0, GL_RGB, GL_FLOAT, pixel);
glTexImage1D(GL_TEXTURE_1D, 2, 3, 2, 0, GL_RGB, GL_FLOAT, pixel);
glTexImage1D(GL_TEXTURE_1D, 3, 3, 1, 0, GL_RGB, GL_FLOAT, pixel);
glTexImage1D(GL_TEXTURE_1D, 4, 3, 0, 0, GL_RGB, GL_FLOAT, pixel);
At the first glTexImage1D call, it would generate a mt with last_level
equal to 3. This would make the later three glTexImage1D call safety.
But when it calling the last glTexImage1D call, the intelTexImage didn't
find a match mt for it, as the level is 4, but the prev mt just with
last_level equal to 3. It then will try to allocate a new mt by calling
intel_miptree_create_for_teximage. It should return NULL as this a NULL
texture. But i915_miptree_layout_2d would set the mt->total_height to 2
instead of 0, that make intel_miptree_create_for_teximage return a
non-NULL mt.
In the same intelTexImage function, it then would call
texImage->Data = intel_miptree_image_map(..) to prepare for submitting
data. Then it would trigger a segfalt error as it's going to access
mt->level[4].x_offset[] at intel_miptree_get_image_offset, as we didn't
setup info for level[4] before.
> seems like we would want to bailout whenever width (or height) is zero.
> If the dimensionality does matter, calling
> _mesa_get_texture_dimensions is wrong. It will return 2 for
> GL_TEXTURE_1D_ARRAY, and I think we want to treat 1D texture arrays
> the same as non-array 1D textures.
Thanks for inforamtion.
>
> > + /* + * This is the _real_ last level of 1D texture
> > with width equal + * to 0. Just simply set total_height to 0
> > to indicate this is + * a NULL texture. Or it will get a
> > value of ALIGN(1, aligh_h), + * which equals to 2, as mesa
> > would set the value of _height_ + * to 1 in 1D texture. +
> > */ + mt->total_height = 0; + return; + } +
> > mt->total_width = mt->width0;
>
> I see that mt->total_height gets set to zero below. I'm move that
> assignment and the mt->total_width = mt->width0 assignment above the
> 'if (width == 0)' condition.
Yes.
While writting this email, I thought another patch to fix this issue.
Will send it later.
Thanks,
Yuanhan Liu
>
> > intel_get_texture_alignment_unit(mt->format, &align_w, &align_h);
> >
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk5pIpcACgkQX1gOwKyEAw9RogCcDK8vXR2bfcjHS4sb24SS8g54
> 8nQAnRcISXoA+oeNkNqoxEvgzOpmJ8Wa
> =nT5n
> -END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Clarify error message about whole-array assignment in GLSL 1.10.
On 8 September 2011 18:06, Eric Anholt wrote: > On Thu, 08 Sep 2011 12:57:31 -0700, Ian Romanick > wrote: > > -BEGIN PGP SIGNED MESSAGE- > > Hash: SHA1 > > > > On 09/07/2011 12:51 PM, Eric Anholt wrote: > > > Previously, it would produce: > > > > > > Failed to compile FS: 0:6(7): error: non-lvalue in assignment > > > > > > and now it produces: > > > > > > Failed to compile FS: 0:5(7): error: whole array assignment is not > > > allowed in GLSL 1.10 or GLSL ES 1.00. > > > > > > Also, add spec quotation to the two places we have code for array > > > lvalues in GLSL 1.10. > > > > There are two places in the code that use is_lvalue. One is in > > do_assignment, and the other is in match_function_by_name (for out and > > inout parameters). Since we've already special case handle whole > > arrays in do_assignment, I think we should do the same in > > match_function_by_name and remove array_lvalue. > > > > In particular, I think we should check that out and inout parameters > > can be arrays when processing the function prototype. That should be > > valid (and "just work") in everything except desktop GLSL 1.10. Right? > > > > That may be a reasonable thing to do as a follow-on patch... > Call me crazy, but I would have sworn that we agreed on this plan when the three of us talked in person on Wednesday. In any case, I'm glad to have the discussion on list, and I agree 100% with this plan. Some of my work on gl_ClipDistance is currently blocked because array_lvalue is set incorrectly for built-in arrays, and Ian's proposal for eliminating array_lvalue seems like the best way to fix it. > > Paul said he needed to do some work on array_lvalue, so I'm leaving that > up to him in a followup patch. I do agree that just doing it in > function parameter handling sounds better than the way we've got it > wired up today. > I don't have any work I need to do on array_lvalue other than the work we are discussing in this thread. I currently have the greatest need for this change (since it's blocking my gl_ClipDistance work) so I'll take it as my task unless I hear otherwise. But if either of the two of you would rather do it just say so--you both are way more familiar with the code base than I am and can probably do it much faster than I can. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] [PATCH 2/2] i965: setup the edge flag enable bit in VE on SNB+
On Thu, Sep 08, 2011 at 08:39:46AM -0700, Eric Anholt wrote: > On Thu, 8 Sep 2011 11:00:52 +0800, Yuanhan Liu > wrote: > > This patch is just for RFC, as I am not sure it's the right way to setup > > the edge flag enable bit in Vertex Element struture. Setting up this > > bit, according to Bspec, need do: > > 1. Edge flags are supported for the following primitives > > 3DPRIM_TRILIST* > > 3DPRIM_TRISTRIP* > > 3DPRIM_TRIFAN* > > 3DPRIM_POLYGON > > 2. This bit must only be ENABLED on the last valid VERTEX_ELEMENT > > structure. > > > > 3. When set, Component 0 Control must be set to VFCOMP_STORE_SRC, and > > Component 1-3 Control must be set to VFCOMP_NOSTORE. > > > > 4. The Source Element Format must be set to the UINT format. > > > > This patch did 1, 2, but didn't do 3, 4. As it simply seems wrong to me > > just change the last vetex element's component setting and source > > element format. > > > > Thoughts? > > > > BTW, this patch fix the oglc pntrast fail on SNB(haven't tested it on > > IVB yet). > > Could you include a piglit test for the failure? Actually, this is an issue of edgeflag. You will also find this patch will fix the oglc edgeflag test fail. pntrast test case would also use polygonmode with edgeflag to render a point. Thus it failed. I simply grep-ed the piglit repo, didn't find any references on glEdgeFlag. Seems that current piglit doesn't include this test? > > > + /* > > + * According to Bspec, the edge flag enable bit should be set > > + * at the last valid vertex element structure > > + */ > > Instead of saying "According to the Bspec", could you actually cite the > text from the public PRM? For example, in one of my previous patches: Thanks for the suggestions. Yeah, I should and will do this. Besides this, any ideas on how to setup the edge flag enable bit rightly to match the Bspec? Or, does this patch make sense to you? Thanks, Yuanhan Liu > > ... > + * And this last workaround is tricky because of the requirements on > + * that bit. From section 1.4.7.2.3 "Stall" of the Sandy Bridge PRM > + * volume 2 part 1: > + * > + * "1 of the following must also be set: > + * - Render Target Cache Flush Enable ([12] of DW1) > + * - Depth Cache Flush Enable ([0] of DW1) > + * - Stall at Pixel Scoreboard ([1] of DW1) > + * - Depth Stall ([13] of DW1) > + * - Post-Sync Operation ([13] of DW1) > + * - Notify Enable ([8] of DW1)" > > It means that someone else stumbling on this code later gets a pointer > to where to start reading up on what's going on in the code. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Clarify error message about whole-array assignment in GLSL 1.10.
On Thu, 08 Sep 2011 12:57:31 -0700, Ian Romanick wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 09/07/2011 12:51 PM, Eric Anholt wrote: > > Previously, it would produce: > > > > Failed to compile FS: 0:6(7): error: non-lvalue in assignment > > > > and now it produces: > > > > Failed to compile FS: 0:5(7): error: whole array assignment is not > > allowed in GLSL 1.10 or GLSL ES 1.00. > > > > Also, add spec quotation to the two places we have code for array > > lvalues in GLSL 1.10. > > There are two places in the code that use is_lvalue. One is in > do_assignment, and the other is in match_function_by_name (for out and > inout parameters). Since we've already special case handle whole > arrays in do_assignment, I think we should do the same in > match_function_by_name and remove array_lvalue. > > In particular, I think we should check that out and inout parameters > can be arrays when processing the function prototype. That should be > valid (and "just work") in everything except desktop GLSL 1.10. Right? > > That may be a reasonable thing to do as a follow-on patch... Paul said he needed to do some work on array_lvalue, so I'm leaving that up to him in a followup patch. I do agree that just doing it in function parameter handling sounds better than the way we've got it wired up today. pgpC4uI4dHTgg.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
Am 08.09.2011 21:43, schrieb Ian Romanick: > On 09/08/2011 12:18 PM, Roland Scheidegger wrote: >> Am 08.09.2011 21:03, schrieb Roland Scheidegger: >>> Am 08.09.2011 19:53, schrieb Ian Romanick: On 09/06/2011 03:21 PM, Roland Scheidegger wrote: > Am 06.09.2011 22:13, schrieb Ian Romanick: >> From: Ian Romanick >> >> Core Mesa already does the dispatch offset remapping for >> every function that could possibly ever be supported. >> There's no need to continue using that cruft in the >> driver. >> >> Since the call to _mesa_enable_imaging_extensions (via >> driInitExtensions) is removed, EXT_blend_logic_op is >> explicitly added to the list. EXT_blend_color is also >> added, but it depends on the drmSupportsBlendColor flag. > Hmm, I don't think EXT_blend_logic_op was advertized before. > The reason for this is that EXT_blend_logic_op together with EXT_blend_logic_op *was* previously enabled. r200CreateContext called driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE parameter tells driInitExtensions to call _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in turn enables: GL_EXT_blend_color GL_EXT_blend_logic_op GL_EXT_blend_minmax GL_EXT_blend_subtract >>> That's right. _mesa_enable_imaging_extensions however did not >>> always enable EXT_blend_logic_op. I was curious (as I was sure it >>> was correct at one point for r200) when this got broken for r200 >>> and that's the answer: 6775c1e8ccc2c543d97eb273a342140a62d99aee - >>> that is OLD (interestingly the commit mentions some discussion >>> apparently however I did not realize it in fact made r200 >>> advertize EXT_blend_logic_op which I knew would be incorrect). >>> I didn't see anything in r200_state.c to handle blend equation being set to GL_LOGIC_OP. >>> Yes - there was code initially handling this (trivial as long as >>> it's the same on all RGBA channels) but I removed that a decade >>> or so ago when adding support for EXT_blend_equation_separate >>> (and removing support for EXT_blend_logic_op at the same time). >>> Of course, we have *zero* piglit tests for this extension. > EXT_blend_equation_separate allows some unholy combinations > which the r200 (possibly other hw too) can't handle > correctly. Namely this combination makes it possible to have > logic ops on rgb or alpha channels and color blending on the > other channels. I know that at least sometime in the past > this driver did not advertize EXT_blend_logic_op, since > OpenGL 1.1 style logic ops do not have that problem and > EXT_blend_logic_op wasn't really all that important. I guess > though it's not exactly a severe problem since surely apps > old enough to use EXT_blend_logic_op wouldn't try to use > EXT_blend_equation_separate (though in theory some app could > be clever and really want to do that...). That's a good point. I suspect that no hardware actually handles this case correctly. I seem to recall that this is the reason NVIDIA doesn't support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro cards don't support it, anyway. Does this work on later chips in the Radeon family? I don't think anyone will miss GL_EXT_blend_logic_op if we just remove it altogether. >>> >>> I don't think it works at least for r300. FWIW there's a mesa >>> helper function (_mesa_rgba_logicop_enabled) which also only >>> makes sense if the logicop blend equation is set for either both >>> of none of RGB/A. >>> >>> I certainly wouldn't mourn the loss of EXT_blend_logic_op. > > >> Hmm actually a quick search of ARB_color_imaging reveals that >> EXT_blend_logic_op isn't even part of it (not that it matters much >> as we don't support the imaging subset any longer anyway) so I >> don't know why it got enabled there. In any case since that imaging >> subset enable is gone, it's not really a problem anymore and >> drivers can just enable it if they really want - r200 certainly >> does not. > > Okay. I'll disable it in r200 and r300. With that change, do I have > your Reviewed-by? Yes otherwise looks good. Given that r300 (but not r200) actually can handle logicop blend equation (just not with separate logicop blend equation) and the fact that mesa due to incorrect implementation of EXT_blend_logic_op actually will never do separate logicop blend equation I don't know what the correct answer there is. Looks like 3 ways possible: 1) leave the bogus mesa logicop check in and hence drivers like r300 can still claim support for EXT_blend_logic_op. This is incorrect wrt to spec but might allow some REALLY old apps requiring this extension to run (could also reimplement it for r200 if anyone is interested...) 2) remove EXT_blend_logic_op completely. 3) fix mesa EXT_blend_logic_op/EXT_blend_equation_separate interaction
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
Am 08.09.2011 23:13, schrieb Marek Olšák: > On Thu, Sep 8, 2011 at 9:03 PM, Roland Scheidegger wrote: >> Am 08.09.2011 19:53, schrieb Ian Romanick: >>> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: Am 06.09.2011 22:13, schrieb Ian Romanick: > From: Ian Romanick > > Core Mesa already does the dispatch offset remapping for every > function that could possibly ever be supported. There's no need to > continue using that cruft in the driver. > > Since the call to _mesa_enable_imaging_extensions (via > driInitExtensions) is removed, EXT_blend_logic_op is explicitly added > to the list. EXT_blend_color is also added, but it depends on the > drmSupportsBlendColor flag. >>> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason for this is that EXT_blend_logic_op together with >>> >>> EXT_blend_logic_op *was* previously enabled. r200CreateContext called >>> driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE >>> parameter tells driInitExtensions to call >>> _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in >>> turn enables: >>> >>>GL_EXT_blend_color >>>GL_EXT_blend_logic_op >>>GL_EXT_blend_minmax >>>GL_EXT_blend_subtract >> That's right. _mesa_enable_imaging_extensions however did not always >> enable EXT_blend_logic_op. >> I was curious (as I was sure it was correct at one point for r200) when >> this got broken for r200 and that's the answer: >> 6775c1e8ccc2c543d97eb273a342140a62d99aee - that is OLD (interestingly >> the commit mentions some discussion apparently however I did not realize >> it in fact made r200 advertize EXT_blend_logic_op which I knew would be >> incorrect). >> >>> >>> I didn't see anything in r200_state.c to handle blend equation being set >>> to GL_LOGIC_OP. >> Yes - there was code initially handling this (trivial as long as it's >> the same on all RGBA channels) but I removed that a decade or so ago >> when adding support for EXT_blend_equation_separate (and removing >> support for EXT_blend_logic_op at the same time). >> >>> >>> Of course, we have *zero* piglit tests for this extension. >>> EXT_blend_equation_separate allows some unholy combinations which the r200 (possibly other hw too) can't handle correctly. Namely this combination makes it possible to have logic ops on rgb or alpha channels and color blending on the other channels. I know that at least sometime in the past this driver did not advertize EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that problem and EXT_blend_logic_op wasn't really all that important. I guess though it's not exactly a severe problem since surely apps old enough to use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate (though in theory some app could be clever and really want to do that...). >>> >>> That's a good point. I suspect that no hardware actually handles this >>> case correctly. I seem to recall that this is the reason NVIDIA doesn't >>> support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro >>> cards don't support it, anyway. >>> >>> Does this work on later chips in the Radeon family? >>> >>> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove >>> it altogether. >> >> I don't think it works at least for r300. FWIW there's a mesa helper >> function (_mesa_rgba_logicop_enabled) which also only makes sense if the >> logicop blend equation is set for either both of none of RGB/A. >> >> I certainly wouldn't mourn the loss of EXT_blend_logic_op. > > Gallium implements blend_logic_op in terms of pure GL1.1 logic op. Assuming > that's incorrect, we shouldn't advertise that extension for Gallium at > all FWIW. This sounds correct - I vaguely remember that we found noone really needs separate RGB/A logic op blend equation (certainly not d3d it's purely there for OpenGL) so that's how it was implemented. Though actually mesa doesn't implement EXT_blend_logic_op correctly anyway. It will prevent using GL_LOGIC_OP with blend_equation_separate (see legal_blend_equation function). This directly contradicts the dependency section of EXT_blend_equation_separate so seems wrong. Though on the upside it means drivers don't really need to care :-). Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 40729] New: d3d1x build error: any interface changes lately?
https://bugs.freedesktop.org/show_bug.cgi?id=40729 Summary: d3d1x build error: any interface changes lately? Product: Mesa Version: git Platform: All OS/Version: All Status: NEW Severity: major Priority: medium Component: Other AssignedTo: mesa-dev@lists.freedesktop.org ReportedBy: alexandre.f.dem...@gmail.com g++ -c -I. -I../../../../../src/gallium/include -I../../../../../src/gallium/auxiliary -I../../../../../src/gallium/drivers -I../../../../../include -Iinclude -I../gd3dapi -I../d3dapi -I../w32api -I../d3d1xstutil/include -I../include -I../../../include -I../../../auxiliary -I../../../state_trackers/egl/common -g -O2 -Wall -fno-strict-aliasing -m64 -g -fPIC -D_GNU_SOURCE -DPTHREADS -DDEBUG -DTEXTURE_FLOAT_ENABLED -DHAVE_POSIX_MEMALIGN -DMESA_SELINUX -DUSE_XCB -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DGLX_USE_TLS -DPTHREADS -DUSE_EXTERNAL_DXTN_LIB=1 -DIN_DRI_DRIVER -DHAVE_ALIAS -DHAVE_MINCORE -DHAVE_LIBUDEV -DHAVE_XCB_DRI2 -DXCB_DRI2_CONNECT_DEVICE_NAME_BROKEN -DHAVE_XEXTPROTO_71 -D__STDC_CONSTANT_MACROS -DHAVE_LLVM=0x0208 -fvisibility=hidden -DDXGI_DRIVER_SEARCH_DIR=\"/usr/lib/x86_64-linux-gnu/egl\" -I/usr/lib/llvm-2.8/include -D_GNU_SOURCE -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DGALLIUM_DXGI_USE_X11 -DGALLIUM_DXGI_USE_DRM src/dxgi_native.cpp -o src/dxgi_native.o In file included from src/dxgi_private.h:34:0, from src/dxgi_native.cpp:27: ../d3d1xstutil/include/d3d1xstutil.h:250:0: warning: "__uuidof" redefined ../w32api/guiddef.h:50:0: note: this is the location of the previous definition In file included from ../w32api/windef.h:252:0, from ../w32api/windows.h:41, from ../w32api/rpc.h:29, from ../w32api/objbase.h:19, from ../d3d1xstutil/include/d3d1xstutil.h:45, from src/dxgi_private.h:34, from src/dxgi_native.cpp:27: ../w32api/winnt.h:685:35: warning: attributes ignored on elaborated-type-specifier that is not a forward declaration In file included from ../w32api/objbase.h:20:0, from ../d3d1xstutil/include/d3d1xstutil.h:45, from src/dxgi_private.h:34, from src/dxgi_native.cpp:27: ../w32api/rpcndr.h:177:1: warning: ‘_MIDL_STUB_MESSAGE’ has a field ‘_MIDL_STUB_MESSAGE::SavedContextHandles’ whose type uses the anonymous namespace ../w32api/rpcndr.h:479:32: warning: ‘_SCONTEXT_QUEUE’ has a field ‘_SCONTEXT_QUEUE::ArrayOfObjects’ whose type uses the anonymous namespace src/dxgi_native.cpp: In member function ‘virtual HRESULT GalliumDXGISwapChain::Present(UINT, UINT)’: src/dxgi_native.cpp:1240:46: error: cannot convert ‘native_attachment’ to ‘const native_present_control*’ in argument passing make[5]: *** [src/dxgi_native.o] Error 1 -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] i965/fs: Implement texelFetch() on Gen4.
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/brw_fs_emit.cpp|5 +
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 15 +++
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index 906c158..f742e84 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -295,6 +295,11 @@ fs_visitor::generate_tex(fs_inst *inst, struct brw_reg
dst, struct brw_reg src)
assert(inst->mlen == 7 || inst->mlen == 10);
msg_type = BRW_SAMPLER_MESSAGE_SIMD8_SAMPLE_GRADIENTS;
break;
+ case FS_OPCODE_TXF:
+assert(inst->mlen == 9);
+msg_type = BRW_SAMPLER_MESSAGE_SIMD16_LD;
+simd_mode = BRW_SAMPLER_SIMD_MODE_SIMD16;
+break;
case FS_OPCODE_TXS:
assert(inst->mlen == 3);
msg_type = BRW_SAMPLER_MESSAGE_SIMD16_RESINFO;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 31a2297..01ffde5 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -678,17 +678,23 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
* instructions. We'll need to do SIMD16 here.
*/
simd16 = true;
- assert(ir->op == ir_txb || ir->op == ir_txl);
+ assert(ir->op == ir_txb || ir->op == ir_txl || ir->op == ir_txf);
for (int i = 0; i < ir->coordinate->type->vector_elements; i++) {
fs_inst *inst = emit(BRW_OPCODE_MOV, fs_reg(MRF,
-base_mrf + mlen + i * 2),
+base_mrf + mlen + i * 2,
+coordinate.type),
coordinate);
if (i < 3 && c->key.gl_clamp_mask[i] & (1 << sampler))
inst->saturate = true;
coordinate.reg_offset++;
}
+ /* Initialize the rest of u/v/r with 0.0, for safety */
+ for (int i = ir->coordinate->type->vector_elements; i < 3; i++) {
+emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen + i * 2),
fs_reg(0.0f));
+ }
+
/* lod/bias appears after u/v/r. */
mlen += 6;
@@ -698,7 +704,8 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
mlen++;
} else {
ir->lod_info.lod->accept(this);
-emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen), this->result);
+emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen, this->result.type),
+ this->result);
mlen++;
}
@@ -737,7 +744,7 @@ fs_visitor::emit_texture_gen4(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
inst = emit(FS_OPCODE_TXS, dst);
break;
case ir_txf:
- assert(!"GLSL 1.30 features unsupported");
+ inst = emit(FS_OPCODE_TXF, dst);
break;
}
inst->base_mrf = base_mrf;
--
1.7.6.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] i965/fs: Implement texelFetch() on Ivybridge.
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 23 ---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index d413bc4..31a2297 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -958,12 +958,29 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
mlen += reg_width;
break;
case ir_txf:
- assert(!"GLSL 1.30 features unsupported");
+ /* Unfortunately, the parameters for LD are intermixed: u, lod, v, r. */
+ fs_inst *inst = emit(BRW_OPCODE_MOV,
+ fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
+ coordinate);
+ coordinate.reg_offset++;
+ mlen += reg_width;
+
+ ir->lod_info.lod->accept(this);
+ emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
this->result);
+ mlen += reg_width;
+
+ for (int i = 1; i < ir->coordinate->type->vector_elements; i++) {
+fs_inst *inst = emit(BRW_OPCODE_MOV,
+ fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_D),
+ coordinate);
+coordinate.reg_offset++;
+mlen += reg_width;
+ }
break;
}
/* Set up the coordinate (except for TXD where it was done earlier) */
- if (ir->op != ir_txd && ir->op != ir_txs) {
+ if (ir->op != ir_txd && ir->op != ir_txs && ir->op != ir_txf) {
for (int i = 0; i < ir->coordinate->type->vector_elements; i++) {
fs_inst *inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, base_mrf + mlen),
coordinate);
@@ -981,7 +998,7 @@ fs_visitor::emit_texture_gen7(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
case ir_txb: inst = emit(FS_OPCODE_TXB, dst); break;
case ir_txl: inst = emit(FS_OPCODE_TXL, dst); break;
case ir_txd: inst = emit(FS_OPCODE_TXD, dst); break;
- case ir_txf: assert(!"TXF unsupported."); break;
+ case ir_txf: inst = emit(FS_OPCODE_TXF, dst); break;
case ir_txs: inst = emit(FS_OPCODE_TXS, dst); break;
}
inst->base_mrf = base_mrf;
--
1.7.6.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] i965/fs: Implement texelFetch() on Ironlake and Sandybridge.
Signed-off-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/brw_defines.h |2 ++
src/mesa/drivers/dri/i965/brw_fs.cpp |1 +
src/mesa/drivers/dri/i965/brw_fs.h |1 +
src/mesa/drivers/dri/i965/brw_fs_emit.cpp|4
src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 12 ++--
src/mesa/program/ir_to_mesa.cpp |5 ++---
6 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
index 5f34939..055aa4a 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -633,6 +633,7 @@ enum opcode {
FS_OPCODE_TEX,
FS_OPCODE_TXB,
FS_OPCODE_TXD,
+ FS_OPCODE_TXF,
FS_OPCODE_TXL,
FS_OPCODE_TXS,
FS_OPCODE_DISCARD,
@@ -782,6 +783,7 @@ enum opcode {
#define GEN5_SAMPLER_MESSAGE_SAMPLE_DERIVS 4
#define GEN5_SAMPLER_MESSAGE_SAMPLE_BIAS_COMPARE 5
#define GEN5_SAMPLER_MESSAGE_SAMPLE_LOD_COMPARE 6
+#define GEN5_SAMPLER_MESSAGE_SAMPLE_LD 7
#define GEN5_SAMPLER_MESSAGE_SAMPLE_RESINFO 10
/* for GEN5 only */
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 7f5194b..9a89f88 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -156,6 +156,7 @@ fs_visitor::implied_mrf_writes(fs_inst *inst)
case FS_OPCODE_TEX:
case FS_OPCODE_TXB:
case FS_OPCODE_TXD:
+ case FS_OPCODE_TXF:
case FS_OPCODE_TXL:
case FS_OPCODE_TXS:
return 1;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
b/src/mesa/drivers/dri/i965/brw_fs.h
index f3d8fbf..0bd518f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -291,6 +291,7 @@ public:
return (opcode == FS_OPCODE_TEX ||
opcode == FS_OPCODE_TXB ||
opcode == FS_OPCODE_TXD ||
+ opcode == FS_OPCODE_TXF ||
opcode == FS_OPCODE_TXL ||
opcode == FS_OPCODE_TXS);
}
diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index ba0d2a2..906c158 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -249,6 +249,9 @@ fs_visitor::generate_tex(fs_inst *inst, struct brw_reg dst,
struct brw_reg src)
/* There is no sample_d_c message; comparisons are done manually */
msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_DERIVS;
break;
+ case FS_OPCODE_TXF:
+msg_type = GEN5_SAMPLER_MESSAGE_SAMPLE_LD;
+break;
default:
assert(!"not reached");
break;
@@ -782,6 +785,7 @@ fs_visitor::generate_code()
case FS_OPCODE_TEX:
case FS_OPCODE_TXB:
case FS_OPCODE_TXD:
+ case FS_OPCODE_TXF:
case FS_OPCODE_TXL:
case FS_OPCODE_TXS:
generate_tex(inst, dst, src[0]);
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index ba7ee2f..d413bc4 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -785,7 +785,8 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
for (int i = 0; i < vector_elements; i++) {
fs_inst *inst = emit(BRW_OPCODE_MOV,
- fs_reg(MRF, base_mrf + mlen + i * reg_width),
+ fs_reg(MRF, base_mrf + mlen + i * reg_width,
+ coordinate.type),
coordinate);
if (i < 3 && c->key.gl_clamp_mask[i] & (1 << sampler))
inst->saturate = true;
@@ -861,7 +862,14 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst,
fs_reg coordinate,
inst = emit(FS_OPCODE_TXS, dst);
break;
case ir_txf:
- assert(!"GLSL 1.30 features unsupported");
+ mlen = header_present + 4 * reg_width;
+
+ this->result = reg_undef;
+ ir->lod_info.lod->accept(this);
+ emit(BRW_OPCODE_MOV,
+ fs_reg(MRF, base_mrf + mlen - reg_width, BRW_REGISTER_TYPE_UD),
+ this->result);
+ inst = emit(FS_OPCODE_TXF, dst);
break;
}
inst->base_mrf = base_mrf;
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 9813c4a..ab5de39 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -2139,6 +2139,8 @@ ir_to_mesa_visitor::visit(ir_texture *ir)
ir->lod_info.bias->accept(this);
lod_info = this->result;
break;
+ case ir_txf:
+ /* Pretend to be TXL so the sampler, coordinate, lod are available */
case ir_txl:
opcode = OPCODE_TXL;
ir->lod_info.lod->accept(this);
@@ -2151,9 +2153,6 @@ ir_to_mesa_visitor::visit(ir_texture *ir)
ir->lod_info.grad.dPdy->accept(this);
dy = this->result;
break;
- case ir_txf:
- assert(!"GLSL 1.30 features unsupported");
-
[Mesa-dev] [PATCH 0/3] i965 texelFetch/TXF support
These patches implement texelFetch/TXF via the "ld" sampler message on all i965 platforms. I've tested it on Broadwater, Cantiga, Sandybridge, and Ivybridge. Pretty sure I tested on Ironlake too, but a while ago. Piglit tests fs-texelFetch-2D and fs-texelFetchOffset-2D pass now. Huge thanks to Dave for writing those. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] New VS by default.
On 09/07/2011 01:03 PM, Eric Anholt wrote: > With the core GLSL fixups I just sent out plus this patch series, I > think it's ready to turn on by default. piglit is non-regressing, and > the oglconform I've tested is overall better (but not perfect), and I > think it will fix one actual bug report. > > Performance is likely to be lower on some applications (I'd be > interested in knowing which) until the optimization patches land. Excellent. Really glad to see new VS on by default. Patches 1, 3, and 6 are: Reviewed-by: Kenneth Graunke The rest are: Acked-by: Kenneth Graunke ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
On Thu, Sep 8, 2011 at 9:03 PM, Roland Scheidegger wrote: > Am 08.09.2011 19:53, schrieb Ian Romanick: >> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: >>> Am 06.09.2011 22:13, schrieb Ian Romanick: From: Ian Romanick Core Mesa already does the dispatch offset remapping for every function that could possibly ever be supported. There's no need to continue using that cruft in the driver. Since the call to _mesa_enable_imaging_extensions (via driInitExtensions) is removed, EXT_blend_logic_op is explicitly added to the list. EXT_blend_color is also added, but it depends on the drmSupportsBlendColor flag. >> >>> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason >>> for this is that EXT_blend_logic_op together with >> >> EXT_blend_logic_op *was* previously enabled. r200CreateContext called >> driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE >> parameter tells driInitExtensions to call >> _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in >> turn enables: >> >> GL_EXT_blend_color >> GL_EXT_blend_logic_op >> GL_EXT_blend_minmax >> GL_EXT_blend_subtract > That's right. _mesa_enable_imaging_extensions however did not always > enable EXT_blend_logic_op. > I was curious (as I was sure it was correct at one point for r200) when > this got broken for r200 and that's the answer: > 6775c1e8ccc2c543d97eb273a342140a62d99aee - that is OLD (interestingly > the commit mentions some discussion apparently however I did not realize > it in fact made r200 advertize EXT_blend_logic_op which I knew would be > incorrect). > >> >> I didn't see anything in r200_state.c to handle blend equation being set >> to GL_LOGIC_OP. > Yes - there was code initially handling this (trivial as long as it's > the same on all RGBA channels) but I removed that a decade or so ago > when adding support for EXT_blend_equation_separate (and removing > support for EXT_blend_logic_op at the same time). > >> >> Of course, we have *zero* piglit tests for this extension. >> >>> EXT_blend_equation_separate allows some unholy combinations which the >>> r200 (possibly other hw too) can't handle correctly. Namely this >>> combination makes it possible to have logic ops on rgb or alpha channels >>> and color blending on the other channels. >>> I know that at least sometime in the past this driver did not advertize >>> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that >>> problem and EXT_blend_logic_op wasn't really all that important. I guess >>> though it's not exactly a severe problem since surely apps old enough to >>> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate >>> (though in theory some app could be clever and really want to do that...). >> >> That's a good point. I suspect that no hardware actually handles this >> case correctly. I seem to recall that this is the reason NVIDIA doesn't >> support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro >> cards don't support it, anyway. >> >> Does this work on later chips in the Radeon family? >> >> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove >> it altogether. > > I don't think it works at least for r300. FWIW there's a mesa helper > function (_mesa_rgba_logicop_enabled) which also only makes sense if the > logicop blend equation is set for either both of none of RGB/A. > > I certainly wouldn't mourn the loss of EXT_blend_logic_op. Gallium implements blend_logic_op in terms of pure GL1.1 logic op. Assuming that's incorrect, we shouldn't advertise that extension for Gallium at all FWIW. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
On Thu, 08 Sep 2011 20:13:45 +0200, Roland Scheidegger wrote: > Am 08.09.2011 19:59, schrieb Eric Anholt: > > On Thu, 08 Sep 2011 10:53:45 -0700, Ian Romanick > > wrote: > >> -BEGIN PGP SIGNED MESSAGE- > >> Hash: SHA1 > >> > >> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: > >>> EXT_blend_equation_separate allows some unholy combinations which the > >>> r200 (possibly other hw too) can't handle correctly. Namely this > >>> combination makes it possible to have logic ops on rgb or alpha channels > >>> and color blending on the other channels. > >>> I know that at least sometime in the past this driver did not advertize > >>> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that > >>> problem and EXT_blend_logic_op wasn't really all that important. I guess > >>> though it's not exactly a severe problem since surely apps old enough to > >>> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate > >>> (though in theory some app could be clever and really want to do that...). > >> > >> That's a good point. I suspect that no hardware actually handles this > >> case correctly. I seem to recall that this is the reason NVIDIA doesn't > >> support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro > >> cards don't support it, anyway. > >> > >> Does this work on later chips in the Radeon family? > >> > >> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove > >> it altogether. > > > > Sadly, for the purpose of doing X on top of GL, we actually do want > > logic ops. > > Yes but do we need GL_EXT_blend_logic_op and not the GL 1.1 style logic ops? Oh, I guess what I was looking for was just GL 1.1 logic ops. I assumed that was what this was. None of the apps I have extension usage for used this one, so I wouldn't miss it. pgpLQ6ZA9BmNL.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] New VS by default.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/07/2011 01:03 PM, Eric Anholt wrote: > With the core GLSL fixups I just sent out plus this patch series, > I think it's ready to turn on by default. piglit is > non-regressing, and the oglconform I've tested is overall better > (but not perfect), and I think it will fix one actual bug report. > > Performance is likely to be lower on some applications (I'd be > interested in knowing which) until the optimization patches land. Strong work. The series looks good to me. Reviewed-by: Ian Romanick -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5pJIEACgkQX1gOwKyEAw+WhgCgmSrBz7hgFNKPrk+HlObXsSTh 3IQAn1uL2Tg3i4iuyUoctAFt7RFJam+z =SbMk -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel: fix null 1D texture handling
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 09/08/2011 03:16 AM, Yuanhan Liu wrote:
> If user call glTexImage1D with width = 0 and height = 0(the last
> level) like this: glTexImage1D(GL_TEXTURE_1D, level, interfomart,
> 0, 0, format, type, pixel)
>
> It would generate a SIGSEGV fault. As i945_miptree_layout_2d didn't
> handle this special case. More info are commented in line in this
> patch.
>
> This would fix the oglc divzero(basic.texQOrWEqualsZero) and
> divzero(basic.texTrivialPrim) test case fail.
>
> Signed-off-by: Yuanhan Liu
This feels like a band-aid. Can this problem also occur on pre-i945
hardware (that use i915_miptree_layout_2d)?
> --- src/mesa/drivers/dri/intel/intel_tex_layout.c | 12
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c
> b/src/mesa/drivers/dri/intel/intel_tex_layout.c index
> 9d81523..4ec71c2 100644 ---
> a/src/mesa/drivers/dri/intel/intel_tex_layout.c +++
> b/src/mesa/drivers/dri/intel/intel_tex_layout.c @@ -61,6 +61,18 @@
> void i945_miptree_layout_2d(struct intel_context *intel, GLuint
> width = mt->width0; GLuint height = mt->height0;
>
> + if (width == 0 && _mesa_get_texture_dimensions(mt->target) ==
> 1) {
Does the dimensionality of the texture target actually matter? It
seems like we would want to bailout whenever width (or height) is zero.
If the dimensionality does matter, calling
_mesa_get_texture_dimensions is wrong. It will return 2 for
GL_TEXTURE_1D_ARRAY, and I think we want to treat 1D texture arrays
the same as non-array 1D textures.
> + /* + * This is the _real_ last level of 1D texture
> with width equal + * to 0. Just simply set total_height to 0
> to indicate this is + * a NULL texture. Or it will get a
> value of ALIGN(1, aligh_h), + * which equals to 2, as mesa
> would set the value of _height_ + * to 1 in 1D texture. +
> */ + mt->total_height = 0; + return; + } +
> mt->total_width = mt->width0;
I see that mt->total_height gets set to zero below. I'm move that
assignment and the mt->total_width = mt->width0 assignment above the
'if (width == 0)' condition.
> intel_get_texture_alignment_unit(mt->format, &align_w, &align_h);
>
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk5pIpcACgkQX1gOwKyEAw9RogCcDK8vXR2bfcjHS4sb24SS8g54
8nQAnRcISXoA+oeNkNqoxEvgzOpmJ8Wa
=nT5n
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/20] intel: Silence "intel/intel_fbo.h:105:4: warning: comparison of unsigned expression < 0 is always false"
On Mon, 29 Aug 2011 14:59:02 -0700, "Ian Romanick" wrote: > From: Ian Romanick > > The test was of an enum, attIndex, which should be unsigned. The > explicit check for < 0 was replaced with a cast to unsigned in an > assertion that attIndex is less than the size of the array it will be > used to index. > > --- > src/mesa/drivers/dri/intel/intel_fbo.h |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/drivers/dri/intel/intel_fbo.h > b/src/mesa/drivers/dri/intel/intel_fbo.h > index 2487994..d8fc1a5 100644 > --- a/src/mesa/drivers/dri/intel/intel_fbo.h > +++ b/src/mesa/drivers/dri/intel/intel_fbo.h > @@ -29,6 +29,8 @@ > #define INTEL_FBO_H > > #include > +#include > +#include "main/compiler.h" > #include "main/formats.h" > #include "intel_screen.h" > > @@ -101,9 +103,7 @@ intel_get_renderbuffer(struct gl_framebuffer *fb, > gl_buffer_index attIndex) > struct gl_renderbuffer *rb; > struct intel_renderbuffer *irb; > > - /* XXX: Who passes -1 to intel_get_renderbuffer? */ > - if (attIndex < 0) > - return NULL; > + assert((unsigned)attIndex < Elements(fb->Attachment)); Majority of uses in the driver is the kernel-style ARRAY_SIZE() instead of Elements(), but this is definitely better either way. pgp5YUwgx8AxO.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/20] intel: Silence several "warning: unused parameter"
On Mon, 29 Aug 2011 14:59:00 -0700, "Ian Romanick" wrote: > From: Ian Romanick > > The internalFormat, format, and type parameters were not used by > either try_pbo_upload or try_pbo_zcopy, so remove them. The width > parameter was also not used by try_pbo_zcopy (because it doesn't > actually copy anything), so remove it too. The current structure of this code is so hateful I can't bring myself to say anything about whether changing the current code is good or bad. I have a dream that one call would try to make a surface (miptree/region) out of the PBO, then we'd see about whether it matches up nicely and zero-copy/blit using that. That would be reusable for texsubimage, which is currently awful in this respect. pgpYVaepyVNYb.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/20] intel: Silence "warning: unused parameter ‘depth0’"
On Mon, 29 Aug 2011 14:58:59 -0700, "Ian Romanick" wrote: > From: Ian Romanick > > The depth0 parameter was not used in intel_miptree_create_for_region, > so remove it. All of the places that call this function, pass 1 for > that parameter, and the place where it looks like it should have been > used (the call to intel_miptree_create_internal) also had 1 hard > coded. I haven't heard anything about the various EGL image things doing 3D textures/array textures/whatever soon, so it seems fine to cut it out. pgppVjHMPGsCQ.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/20] intel: Silence "warning: unused parameter ‘target’"
On Mon, 29 Aug 2011 14:58:58 -0700, "Ian Romanick" wrote: > From: Ian Romanick > > The GLenum target parameter was not used in intel_copy_texsubimage, so > remove it. We should also stop having all 3 callers look up the internalFormat from the texImage->InternalFormat and pass it in. pgp5c7qsVkAGX.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/20] intel: Silence "warning: unused parameter ‘fb’"
On Mon, 29 Aug 2011 14:58:56 -0700, "Ian Romanick" wrote: > From: Ian Romanick > > The gl_framebuffer was not used in intel_draw_buffer, so remove it. Reviewed-by: Eric Anholt pgpon7SjIctA3.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/20] intel: Silence several "warning: unused parameter"
On Mon, 29 Aug 2011 14:58:54 -0700, "Ian Romanick" wrote:
> From: Ian Romanick
>
> ---
> src/mesa/drivers/dri/intel/intel_buffer_objects.c | 47
> ++---
> 1 files changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_buffer_objects.c
> b/src/mesa/drivers/dri/intel/intel_buffer_objects.c
> index d908975..0acc3a1 100644
> --- a/src/mesa/drivers/dri/intel/intel_buffer_objects.c
> +++ b/src/mesa/drivers/dri/intel/intel_buffer_objects.c
> @@ -151,6 +151,12 @@ intel_bufferobj_data(struct gl_context * ctx,
> struct intel_context *intel = intel_context(ctx);
> struct intel_buffer_object *intel_obj = intel_buffer_object(obj);
>
> + /* Part of the ABI, but this function doesn't use it.
> +*/
> +#ifndef I915
> + (void) target;
> +#endif
> +
To me, any benefit of parameter cleanups (like below) you might stumble
across from enabling these warnings are outweighed by the ugliness of
adding things like this to mark the warnings as handled.
> intel_obj->Base.Size = size;
> intel_obj->Base.Usage = usage;
>
> @@ -743,9 +749,7 @@ intel_bufferobj_copy_subdata(struct gl_context *ctx,
>
> #if FEATURE_APPLE_object_purgeable
> static GLenum
> -intel_buffer_purgeable(struct gl_context * ctx,
> - drm_intel_bo *buffer,
> - GLenum option)
> +intel_buffer_purgeable(drm_intel_bo *buffer)
> {
> int retained = 0;
>
> @@ -764,7 +768,7 @@ intel_buffer_object_purgeable(struct gl_context * ctx,
>
> intel = intel_buffer_object (obj);
> if (intel->buffer != NULL)
> - return intel_buffer_purgeable (ctx, intel->buffer, option);
> + return intel_buffer_purgeable(intel->buffer);
>
> if (option == GL_RELEASED_APPLE) {
>if (intel->sys_buffer != NULL) {
> @@ -775,10 +779,8 @@ intel_buffer_object_purgeable(struct gl_context * ctx,
>return GL_RELEASED_APPLE;
> } else {
>/* XXX Create the buffer and madvise(MADV_DONTNEED)? */
> - return intel_buffer_purgeable (ctx,
> -
> intel_bufferobj_buffer(intel_context(ctx),
> -intel,
> INTEL_READ),
> - option);
> + return
> intel_buffer_purgeable(intel_bufferobj_buffer(intel_context(ctx),
> +intel, INTEL_READ));
> }
> }
This could stand some of the usual local variable declarations.
pgpxU5P1EyFCz.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/20] intel: Silence many "intel_batchbuffer.h:97:39: warning: comparison between signed and unsigned integer expressions"
On Mon, 29 Aug 2011 14:58:53 -0700, "Ian Romanick" wrote:
> From: Ian Romanick
>
> ---
> src/mesa/drivers/dri/intel/intel_batchbuffer.h |4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.h
> b/src/mesa/drivers/dri/intel/intel_batchbuffer.h
> index fb4134d..90dc0ed 100644
> --- a/src/mesa/drivers/dri/intel/intel_batchbuffer.h
> +++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.h
> @@ -57,9 +57,11 @@ static INLINE uint32_t float_as_int(float f)
> * be passed as structs rather than dwords, but that's a little bit of
> * work...
> */
> -static INLINE GLint
> +static INLINE unsigned
> intel_batchbuffer_space(struct intel_context *intel)
> {
> + assert((intel->batch.state_batch_offset - intel->batch.reserved_space)
> + >= intel->batch.used*4);
> return (intel->batch.state_batch_offset - intel->batch.reserved_space) -
> intel->batch.used*4;
> }
before and after:
textdata bss dec hex filename
903173 263921552 931117 e352d i965_dri.so
924093 263921552 952037 e86e5 i965_dri.so
Granted, this is a debug build, but that's a lot of bloat.
pgptrtmGnEw6g.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Clarify error message about whole-array assignment in GLSL 1.10.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 09/07/2011 12:51 PM, Eric Anholt wrote:
> Previously, it would produce:
>
> Failed to compile FS: 0:6(7): error: non-lvalue in assignment
>
> and now it produces:
>
> Failed to compile FS: 0:5(7): error: whole array assignment is not
> allowed in GLSL 1.10 or GLSL ES 1.00.
>
> Also, add spec quotation to the two places we have code for array
> lvalues in GLSL 1.10.
There are two places in the code that use is_lvalue. One is in
do_assignment, and the other is in match_function_by_name (for out and
inout parameters). Since we've already special case handle whole
arrays in do_assignment, I think we should do the same in
match_function_by_name and remove array_lvalue.
In particular, I think we should check that out and inout parameters
can be arrays when processing the function prototype. That should be
valid (and "just work") in everything except desktop GLSL 1.10. Right?
That may be a reasonable thing to do as a follow-on patch...
In any case, the series is
Reviewed-by: Ian Romanick
> ---
>
> And, of course, because I failed to do a full test run, it broke
> ES. Add even more spec quotations to clarify.
>
> src/glsl/ast_to_hir.cpp | 31 +-- 1
> files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 6ef763c..9b3f746 100644 --- a/src/glsl/ast_to_hir.cpp +++
> b/src/glsl/ast_to_hir.cpp @@ -679,16 +679,20 @@
> do_assignment(exec_list *instructions, struct
> _mesa_glsl_parse_state *state, lhs->variable_referenced()->name);
> error_emitted = true;
>
> + } else if (state->language_version <= 110 &&
> lhs->type->is_array()) { + /* From page 32 (page 38 of the PDF) of
> the GLSL 1.10 spec: + * + *"Other binary or unary
> expressions, non-dereferenced + * arrays, function names,
> swizzles with repeated fields, +* and constants cannot be
> l-values." +*/ + _mesa_glsl_error(&lhs_loc, state, "whole array
> assignment is not " + "allowed in GLSL 1.10 or GLSL ES
> 1.00."); + error_emitted = true; } else if (!lhs->is_lvalue()) {
> _mesa_glsl_error(& lhs_loc, state, "non-lvalue in assignment");
> error_emitted = true; } - - if (state->es_shader &&
> lhs->type->is_array()) { - _mesa_glsl_error(&lhs_loc, state,
> "whole array assignment is not " - "allowed in GLSL ES
> 1.00."); - error_emitted = true; - } }
>
> ir_rvalue *new_rhs = @@ -2072,6 +2076,21 @@
> apply_type_qualifier_to_variable(const struct ast_type_qualifier
> *qual, else var->depth_layout = ir_depth_layout_none;
>
> + /* From page 46 (page 52 of the PDF) of the GLSL ES
> specification: +* +*"Array variables are l-values and
> may be passed to parameters +* declared as out or inout.
> However, they may not be used as +* the target of an
> assignment." +* +* From page 32 (page 38 of the PDF) of the
> GLSL 1.10 spec: +* +*"Other binary or unary
> expressions, non-dereferenced arrays, +* function names,
> swizzles with repeated fields, and constants +* cannot be
> l-values." +* +* So we only mark 1.10 as non-lvalues, and
> check for array +* assignment in 100 specifically in
> do_assignment. +*/ if (var->type->is_array() &&
> state->language_version != 110) { var->array_lvalue = true; }
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk5pHisACgkQX1gOwKyEAw+NmgCgllFFRxoAwoGGTdu3nNsH7ay1
SLEAn1H8WQxY/OBcWYe8cPER/eX8RXOt
=3cur
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/08/2011 12:18 PM, Roland Scheidegger wrote: > Am 08.09.2011 21:03, schrieb Roland Scheidegger: >> Am 08.09.2011 19:53, schrieb Ian Romanick: >>> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: Am 06.09.2011 22:13, schrieb Ian Romanick: > From: Ian Romanick > > Core Mesa already does the dispatch offset remapping for > every function that could possibly ever be supported. > There's no need to continue using that cruft in the > driver. > > Since the call to _mesa_enable_imaging_extensions (via > driInitExtensions) is removed, EXT_blend_logic_op is > explicitly added to the list. EXT_blend_color is also > added, but it depends on the drmSupportsBlendColor flag. >>> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason for this is that EXT_blend_logic_op together with >>> >>> EXT_blend_logic_op *was* previously enabled. r200CreateContext >>> called driInitExtensions( ctx, card_extensions, GL_TRUE );. >>> The GL_TRUE parameter tells driInitExtensions to call >>> _mesa_enable_imaging_extensions. >>> _mesa_enable_imaging_extensions in turn enables: >>> >>> GL_EXT_blend_color GL_EXT_blend_logic_op GL_EXT_blend_minmax >>> GL_EXT_blend_subtract >> That's right. _mesa_enable_imaging_extensions however did not >> always enable EXT_blend_logic_op. I was curious (as I was sure it >> was correct at one point for r200) when this got broken for r200 >> and that's the answer: 6775c1e8ccc2c543d97eb273a342140a62d99aee - >> that is OLD (interestingly the commit mentions some discussion >> apparently however I did not realize it in fact made r200 >> advertize EXT_blend_logic_op which I knew would be incorrect). >> >>> >>> I didn't see anything in r200_state.c to handle blend equation >>> being set to GL_LOGIC_OP. >> Yes - there was code initially handling this (trivial as long as >> it's the same on all RGBA channels) but I removed that a decade >> or so ago when adding support for EXT_blend_equation_separate >> (and removing support for EXT_blend_logic_op at the same time). >> >>> >>> Of course, we have *zero* piglit tests for this extension. >>> EXT_blend_equation_separate allows some unholy combinations which the r200 (possibly other hw too) can't handle correctly. Namely this combination makes it possible to have logic ops on rgb or alpha channels and color blending on the other channels. I know that at least sometime in the past this driver did not advertize EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that problem and EXT_blend_logic_op wasn't really all that important. I guess though it's not exactly a severe problem since surely apps old enough to use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate (though in theory some app could be clever and really want to do that...). >>> >>> That's a good point. I suspect that no hardware actually >>> handles this case correctly. I seem to recall that this is the >>> reason NVIDIA doesn't support GL_EXT_blend_logic_op in their >>> drivers. I know the non-Quadro cards don't support it, >>> anyway. >>> >>> Does this work on later chips in the Radeon family? >>> >>> I don't think anyone will miss GL_EXT_blend_logic_op if we just >>> remove it altogether. >> >> I don't think it works at least for r300. FWIW there's a mesa >> helper function (_mesa_rgba_logicop_enabled) which also only >> makes sense if the logicop blend equation is set for either both >> of none of RGB/A. >> >> I certainly wouldn't mourn the loss of EXT_blend_logic_op. > > > Hmm actually a quick search of ARB_color_imaging reveals that > EXT_blend_logic_op isn't even part of it (not that it matters much > as we don't support the imaging subset any longer anyway) so I > don't know why it got enabled there. In any case since that imaging > subset enable is gone, it's not really a problem anymore and > drivers can just enable it if they really want - r200 certainly > does not. Okay. I'll disable it in r200 and r300. With that change, do I have your Reviewed-by? -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5pGscACgkQX1gOwKyEAw93RwCgiu/i79FyervNFtqdKxJC4NR5 N44An0e1KrPouqyjA/ZnecKGkxMw+8AE =ds06 -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
Am 08.09.2011 21:03, schrieb Roland Scheidegger: > Am 08.09.2011 19:53, schrieb Ian Romanick: >> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: >>> Am 06.09.2011 22:13, schrieb Ian Romanick: From: Ian Romanick Core Mesa already does the dispatch offset remapping for every function that could possibly ever be supported. There's no need to continue using that cruft in the driver. Since the call to _mesa_enable_imaging_extensions (via driInitExtensions) is removed, EXT_blend_logic_op is explicitly added to the list. EXT_blend_color is also added, but it depends on the drmSupportsBlendColor flag. >> >>> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason >>> for this is that EXT_blend_logic_op together with >> >> EXT_blend_logic_op *was* previously enabled. r200CreateContext called >> driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE >> parameter tells driInitExtensions to call >> _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in >> turn enables: >> >>GL_EXT_blend_color >>GL_EXT_blend_logic_op >>GL_EXT_blend_minmax >>GL_EXT_blend_subtract > That's right. _mesa_enable_imaging_extensions however did not always > enable EXT_blend_logic_op. > I was curious (as I was sure it was correct at one point for r200) when > this got broken for r200 and that's the answer: > 6775c1e8ccc2c543d97eb273a342140a62d99aee - that is OLD (interestingly > the commit mentions some discussion apparently however I did not realize > it in fact made r200 advertize EXT_blend_logic_op which I knew would be > incorrect). > >> >> I didn't see anything in r200_state.c to handle blend equation being set >> to GL_LOGIC_OP. > Yes - there was code initially handling this (trivial as long as it's > the same on all RGBA channels) but I removed that a decade or so ago > when adding support for EXT_blend_equation_separate (and removing > support for EXT_blend_logic_op at the same time). > >> >> Of course, we have *zero* piglit tests for this extension. >> >>> EXT_blend_equation_separate allows some unholy combinations which the >>> r200 (possibly other hw too) can't handle correctly. Namely this >>> combination makes it possible to have logic ops on rgb or alpha channels >>> and color blending on the other channels. >>> I know that at least sometime in the past this driver did not advertize >>> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that >>> problem and EXT_blend_logic_op wasn't really all that important. I guess >>> though it's not exactly a severe problem since surely apps old enough to >>> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate >>> (though in theory some app could be clever and really want to do that...). >> >> That's a good point. I suspect that no hardware actually handles this >> case correctly. I seem to recall that this is the reason NVIDIA doesn't >> support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro >> cards don't support it, anyway. >> >> Does this work on later chips in the Radeon family? >> >> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove >> it altogether. > > I don't think it works at least for r300. FWIW there's a mesa helper > function (_mesa_rgba_logicop_enabled) which also only makes sense if the > logicop blend equation is set for either both of none of RGB/A. > > I certainly wouldn't mourn the loss of EXT_blend_logic_op. Hmm actually a quick search of ARB_color_imaging reveals that EXT_blend_logic_op isn't even part of it (not that it matters much as we don't support the imaging subset any longer anyway) so I don't know why it got enabled there. In any case since that imaging subset enable is gone, it's not really a problem anymore and drivers can just enable it if they really want - r200 certainly does not. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
Am 08.09.2011 19:53, schrieb Ian Romanick: > On 09/06/2011 03:21 PM, Roland Scheidegger wrote: >> Am 06.09.2011 22:13, schrieb Ian Romanick: >>> From: Ian Romanick >>> >>> Core Mesa already does the dispatch offset remapping for every >>> function that could possibly ever be supported. There's no need to >>> continue using that cruft in the driver. >>> >>> Since the call to _mesa_enable_imaging_extensions (via >>> driInitExtensions) is removed, EXT_blend_logic_op is explicitly added >>> to the list. EXT_blend_color is also added, but it depends on the >>> drmSupportsBlendColor flag. > >> Hmm, I don't think EXT_blend_logic_op was advertized before. The reason >> for this is that EXT_blend_logic_op together with > > EXT_blend_logic_op *was* previously enabled. r200CreateContext called > driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE > parameter tells driInitExtensions to call > _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in > turn enables: > >GL_EXT_blend_color >GL_EXT_blend_logic_op >GL_EXT_blend_minmax >GL_EXT_blend_subtract That's right. _mesa_enable_imaging_extensions however did not always enable EXT_blend_logic_op. I was curious (as I was sure it was correct at one point for r200) when this got broken for r200 and that's the answer: 6775c1e8ccc2c543d97eb273a342140a62d99aee - that is OLD (interestingly the commit mentions some discussion apparently however I did not realize it in fact made r200 advertize EXT_blend_logic_op which I knew would be incorrect). > > I didn't see anything in r200_state.c to handle blend equation being set > to GL_LOGIC_OP. Yes - there was code initially handling this (trivial as long as it's the same on all RGBA channels) but I removed that a decade or so ago when adding support for EXT_blend_equation_separate (and removing support for EXT_blend_logic_op at the same time). > > Of course, we have *zero* piglit tests for this extension. > >> EXT_blend_equation_separate allows some unholy combinations which the >> r200 (possibly other hw too) can't handle correctly. Namely this >> combination makes it possible to have logic ops on rgb or alpha channels >> and color blending on the other channels. >> I know that at least sometime in the past this driver did not advertize >> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that >> problem and EXT_blend_logic_op wasn't really all that important. I guess >> though it's not exactly a severe problem since surely apps old enough to >> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate >> (though in theory some app could be clever and really want to do that...). > > That's a good point. I suspect that no hardware actually handles this > case correctly. I seem to recall that this is the reason NVIDIA doesn't > support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro > cards don't support it, anyway. > > Does this work on later chips in the Radeon family? > > I don't think anyone will miss GL_EXT_blend_logic_op if we just remove > it altogether. I don't think it works at least for r300. FWIW there's a mesa helper function (_mesa_rgba_logicop_enabled) which also only makes sense if the logicop blend equation is set for either both of none of RGB/A. I certainly wouldn't mourn the loss of EXT_blend_logic_op. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
Am 08.09.2011 19:59, schrieb Eric Anholt: > On Thu, 08 Sep 2011 10:53:45 -0700, Ian Romanick wrote: >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA1 >> >> On 09/06/2011 03:21 PM, Roland Scheidegger wrote: >>> EXT_blend_equation_separate allows some unholy combinations which the >>> r200 (possibly other hw too) can't handle correctly. Namely this >>> combination makes it possible to have logic ops on rgb or alpha channels >>> and color blending on the other channels. >>> I know that at least sometime in the past this driver did not advertize >>> EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that >>> problem and EXT_blend_logic_op wasn't really all that important. I guess >>> though it's not exactly a severe problem since surely apps old enough to >>> use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate >>> (though in theory some app could be clever and really want to do that...). >> >> That's a good point. I suspect that no hardware actually handles this >> case correctly. I seem to recall that this is the reason NVIDIA doesn't >> support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro >> cards don't support it, anyway. >> >> Does this work on later chips in the Radeon family? >> >> I don't think anyone will miss GL_EXT_blend_logic_op if we just remove >> it altogether. > > Sadly, for the purpose of doing X on top of GL, we actually do want > logic ops. Yes but do we need GL_EXT_blend_logic_op and not the GL 1.1 style logic ops? Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
On Thu, 08 Sep 2011 10:53:45 -0700, Ian Romanick wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 09/06/2011 03:21 PM, Roland Scheidegger wrote: > > EXT_blend_equation_separate allows some unholy combinations which the > > r200 (possibly other hw too) can't handle correctly. Namely this > > combination makes it possible to have logic ops on rgb or alpha channels > > and color blending on the other channels. > > I know that at least sometime in the past this driver did not advertize > > EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that > > problem and EXT_blend_logic_op wasn't really all that important. I guess > > though it's not exactly a severe problem since surely apps old enough to > > use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate > > (though in theory some app could be clever and really want to do that...). > > That's a good point. I suspect that no hardware actually handles this > case correctly. I seem to recall that this is the reason NVIDIA doesn't > support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro > cards don't support it, anyway. > > Does this work on later chips in the Radeon family? > > I don't think anyone will miss GL_EXT_blend_logic_op if we just remove > it altogether. Sadly, for the purpose of doing X on top of GL, we actually do want logic ops. pgpgKmBYLgNEg.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] r200: Enable extensions by just setting the flags
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/06/2011 03:21 PM, Roland Scheidegger wrote: > Am 06.09.2011 22:13, schrieb Ian Romanick: >> From: Ian Romanick >> >> Core Mesa already does the dispatch offset remapping for every >> function that could possibly ever be supported. There's no need to >> continue using that cruft in the driver. >> >> Since the call to _mesa_enable_imaging_extensions (via >> driInitExtensions) is removed, EXT_blend_logic_op is explicitly added >> to the list. EXT_blend_color is also added, but it depends on the >> drmSupportsBlendColor flag. > > Hmm, I don't think EXT_blend_logic_op was advertized before. The reason > for this is that EXT_blend_logic_op together with EXT_blend_logic_op *was* previously enabled. r200CreateContext called driInitExtensions( ctx, card_extensions, GL_TRUE );. The GL_TRUE parameter tells driInitExtensions to call _mesa_enable_imaging_extensions. _mesa_enable_imaging_extensions in turn enables: GL_EXT_blend_color GL_EXT_blend_logic_op GL_EXT_blend_minmax GL_EXT_blend_subtract I didn't see anything in r200_state.c to handle blend equation being set to GL_LOGIC_OP. Of course, we have *zero* piglit tests for this extension. > EXT_blend_equation_separate allows some unholy combinations which the > r200 (possibly other hw too) can't handle correctly. Namely this > combination makes it possible to have logic ops on rgb or alpha channels > and color blending on the other channels. > I know that at least sometime in the past this driver did not advertize > EXT_blend_logic_op, since OpenGL 1.1 style logic ops do not have that > problem and EXT_blend_logic_op wasn't really all that important. I guess > though it's not exactly a severe problem since surely apps old enough to > use EXT_blend_logic_op wouldn't try to use EXT_blend_equation_separate > (though in theory some app could be clever and really want to do that...). That's a good point. I suspect that no hardware actually handles this case correctly. I seem to recall that this is the reason NVIDIA doesn't support GL_EXT_blend_logic_op in their drivers. I know the non-Quadro cards don't support it, anyway. Does this work on later chips in the Radeon family? I don't think anyone will miss GL_EXT_blend_logic_op if we just remove it altogether. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5pASUACgkQX1gOwKyEAw/3OgCfR/mU4O+4dEeGdr6zGrpx7KzU 1zQAnRXhbDF6/gvIFmePr86ddVzKVPVV =OI6v -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] State tracker pipe_surface woes...
Am 08.09.2011 09:08, schrieb Marek Olšák: > 2011/9/7 Roland Scheidegger : >> That said I'm not really sure why pipe_sampler_view and pipe_surface >> actually have a context pointer in them, since they are only supposed to >> be used with the context in which they were created I think those >> shouldn't actually exist - surface_destroy and sampler_view_destroy will >> have context as a parameter, and if they aren't shared between contexts >> it's pointless to store the context pointer. Might be a relic from when >> those structs were created/destroyed using screen functions... > > The ctx pointer is there because of pipe_surface_reference and > pipe_sampler_view_reference. When the refcount becomes 0, the > corresponding pipe...reference function uses the ctx pointer to do > ctx->...destroy(ctx, ... > > So that we can destroy objects just by: > pipe_surface_reference(&surface, NULL); > > This ctx pointer also ensures that the destroy function is called with > the right context. Yes. It's arguable though that these really need refcounting - the state tracker could probably create/destroy them like any other object (e.g. they could follow just the normal create/bind/destroy pattern). Only resources really need refcounting. Even with refcounting you could just pass in the context probably, since the context when creating/destroying them always has to be the same anyway. (pipe_surface though definitely needed refcounting in the past because it was a sharable screen entity once used for things like gl system framebuffers which were not backed by a resource.) It works for now though so I'm not really proposing to change it. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] [PATCH 2/2] i965: setup the edge flag enable bit in VE on SNB+
On Thu, 8 Sep 2011 11:00:52 +0800, Yuanhan Liu wrote: > This patch is just for RFC, as I am not sure it's the right way to setup > the edge flag enable bit in Vertex Element struture. Setting up this > bit, according to Bspec, need do: > 1. Edge flags are supported for the following primitives > 3DPRIM_TRILIST* > 3DPRIM_TRISTRIP* > 3DPRIM_TRIFAN* > 3DPRIM_POLYGON > 2. This bit must only be ENABLED on the last valid VERTEX_ELEMENT > structure. > > 3. When set, Component 0 Control must be set to VFCOMP_STORE_SRC, and > Component 1-3 Control must be set to VFCOMP_NOSTORE. > > 4. The Source Element Format must be set to the UINT format. > > This patch did 1, 2, but didn't do 3, 4. As it simply seems wrong to me > just change the last vetex element's component setting and source > element format. > > Thoughts? > > BTW, this patch fix the oglc pntrast fail on SNB(haven't tested it on > IVB yet). Could you include a piglit test for the failure? > + /* > + * According to Bspec, the edge flag enable bit should be set > + * at the last valid vertex element structure > + */ Instead of saying "According to the Bspec", could you actually cite the text from the public PRM? For example, in one of my previous patches: ... + * And this last workaround is tricky because of the requirements on + * that bit. From section 1.4.7.2.3 "Stall" of the Sandy Bridge PRM + * volume 2 part 1: + * + * "1 of the following must also be set: + * - Render Target Cache Flush Enable ([12] of DW1) + * - Depth Cache Flush Enable ([0] of DW1) + * - Stall at Pixel Scoreboard ([1] of DW1) + * - Depth Stall ([13] of DW1) + * - Post-Sync Operation ([13] of DW1) + * - Notify Enable ([8] of DW1)" It means that someone else stumbling on this code later gets a pointer to where to start reading up on what's going on in the code. pgpys1TdQ9wll.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel: fix null 1D texture handling
If user call glTexImage1D with width = 0 and height = 0(the
last level) like this:
glTexImage1D(GL_TEXTURE_1D, level, interfomart, 0, 0, format, type, pixel)
It would generate a SIGSEGV fault. As i945_miptree_layout_2d
didn't handle this special case. More info are commented in line
in this patch.
This would fix the oglc divzero(basic.texQOrWEqualsZero) and
divzero(basic.texTrivialPrim) test case fail.
Signed-off-by: Yuanhan Liu
---
src/mesa/drivers/dri/intel/intel_tex_layout.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c
b/src/mesa/drivers/dri/intel/intel_tex_layout.c
index 9d81523..4ec71c2 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_layout.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_layout.c
@@ -61,6 +61,18 @@ void i945_miptree_layout_2d(struct intel_context *intel,
GLuint width = mt->width0;
GLuint height = mt->height0;
+ if (width == 0 && _mesa_get_texture_dimensions(mt->target) == 1) {
+ /*
+ * This is the _real_ last level of 1D texture with width equal
+ * to 0. Just simply set total_height to 0 to indicate this is
+ * a NULL texture. Or it will get a value of ALIGN(1, aligh_h),
+ * which equals to 2, as mesa would set the value of _height_
+ * to 1 in 1D texture.
+ */
+ mt->total_height = 0;
+ return;
+ }
+
mt->total_width = mt->width0;
intel_get_texture_alignment_unit(mt->format, &align_w, &align_h);
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] st/egl: correctly return configs under wayland
On Thu, Sep 8, 2011 at 3:53 PM, Benjamin Franzke wrote: > 2011/9/8 Chia-I Wu : >> On Thu, Sep 8, 2011 at 3:11 PM, Benjamin Franzke >> wrote: >>> First thanks for taking this on. >>> >>> There are some things I'd like to have addtionally/differently: >>> >>> Supported shm formats are exposed via a "format" event as well >>> (like the supported drm formats), so the config creation logic is the >>> same for drm and shm, and I think it can remain in native_wayland.c >>> >>> We need roundtrips to check that we get at least one supported format. >>> >>> I've attached two patches (heavily based on your last two) that do this, >>> are you ok with using them? >> That is great. Sure. >> >> I've noted two minor issues or typos, fixed by the attached patch. >> One is that param_premultiplied_alpha can be set in the common code >> and only when the display has both HAS_ARGB32 and HAS_PREMUL_ARGB32. >> The other is that we should not claim PIPE_FORMAT_B8G8R8A8_UNORM >> without HAS_ARGB32. If it looks good to you, I will commit an updated >> version of your patches. > > Ok I see the point, though i can imagine compositors may want > to expose only premultilplied argb, which egl users couldnt use then, > but your right, we shouldnt expose alpha at all if one isnt available. > So I'm fine with you commiting that change. Yeah, another restriction coming from EGL_VG_ALPHA_FORMAT_PRE_BIT and how native.h models it. Anyway, we can adapt when there is a need. -- o...@lunarg.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/5] st/egl: add premultiplied alpha support to wayland
2011/9/8 Chia-I Wu :
> On Thu, Sep 8, 2011 at 3:13 PM, Benjamin Franzke
> wrote:
>> 2011/9/8 Chia-I Wu :
>>> From: Chia-I Wu
>>>
>>> Return true for NATIVE_PARAM_PREMULTIPLIED_ALPHA when all formats with
>>> alpha support premultiplied alpha. Currently, it means when argb32 and
>>> argb32_pre are both supported.
>>> ---
>>> .../state_trackers/egl/wayland/native_drm.c | 8 ++--
>>> .../state_trackers/egl/wayland/native_shm.c | 6 +-
>>> .../state_trackers/egl/wayland/native_wayland.c | 18
>>> ++
>>> .../state_trackers/egl/wayland/native_wayland.h | 3 +++
>>> 4 files changed, 32 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> index facab32..e177e7c 100644
>>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> @@ -114,8 +114,8 @@ wayland_create_drm_buffer(struct wayland_display
>>> *display,
>>>
>>> switch (surface->color_format) {
>>> case PIPE_FORMAT_B8G8R8A8_UNORM:
>>> - /* assume premultiplied */
>>> - format = WL_DRM_FORMAT_PREMULTIPLIED_ARGB32;
>>> + format = (surface->premultiplied_alpha) ?
>>> + WL_DRM_FORMAT_PREMULTIPLIED_ARGB32 : WL_DRM_FORMAT_ARGB32;
>>> break;
>>> case PIPE_FORMAT_B8G8R8X8_UNORM:
>>> format = WL_DRM_FORMAT_XRGB32;
>>> @@ -255,6 +255,10 @@ wayland_drm_display_init_screen(struct native_display
>>> *ndpy)
>>> if (!wayland_drm_display_add_configs(drmdpy))
>>> return FALSE;
>>>
>>> + /* check that premultiplied alpha is supported for all formats with
>>> alpha */
>>> + if (!drmdpy->argb32 || drmdpy->argb32_pre)
>>> + drmdpy->base.param_premultiplied_alpha = TRUE;
>>
>> Why enable premultiplied alpha if argb32 is not exposed?
>> What isnt covered with just: "if (drmdpy->argb32_pre)"?.
> Yes, it is simpler. What I intended to do is to enable pre-multiplied
> alpha when all formats that have alpha also support pre-multiplied
> alpha. In the case there is no format that has alpha
> (!drmdpy->argb32), pre-multiplied alpha can be enabled.
Ok, I see, my idea was to enable only whats actually useful for a configuration,
but I dont know whats the general egl rule here. So I'm ok with both.
>
>>> +
>>> drmdpy->base.base.screen =
>>> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
>>> NULL, drmdpy->fd);
>>> diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c
>>> b/src/gallium/state_trackers/egl/wayland/native_shm.c
>>> index 5882e74..e2d2437 100644
>>> --- a/src/gallium/state_trackers/egl/wayland/native_shm.c
>>> +++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
>>> @@ -95,7 +95,8 @@ wayland_create_shm_buffer(struct wayland_display *display,
>>>
>>> switch (surface->color_format) {
>>> case PIPE_FORMAT_B8G8R8A8_UNORM:
>>> - format = WL_SHM_FORMAT_PREMULTIPLIED_ARGB32;
>>> + format = (surface->premultiplied_alpha) ?
>>> + WL_SHM_FORMAT_PREMULTIPLIED_ARGB32 : WL_SHM_FORMAT_ARGB32;
>>> break;
>>> case PIPE_FORMAT_B8G8R8X8_UNORM:
>>> format = WL_SHM_FORMAT_XRGB32;
>>> @@ -165,6 +166,9 @@ wayland_shm_display_init_screen(struct native_display
>>> *ndpy)
>>> if (!wayland_shm_display_add_configs(shmdpy))
>>> return FALSE;
>>>
>>> + /* assume all formats are supported */
>>> + shmdpy->base.param_premultiplied_alpha = TRUE;
>>> +
>>> winsys = wayland_create_sw_winsys(shmdpy->base.dpy);
>>> if (!winsys)
>>> return FALSE;
>>> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>>> b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>>> index 14cc908..b2dab8f 100644
>>> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>>> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>>> @@ -60,9 +60,13 @@ static int
>>> wayland_display_get_param(struct native_display *ndpy,
>>> enum native_param_type param)
>>> {
>>> + struct wayland_display *display = wayland_display(ndpy);
>>> int val;
>>>
>>> switch (param) {
>>> + case NATIVE_PARAM_PREMULTIPLIED_ALPHA:
>>> + val = display->param_premultiplied_alpha;
>>> + break;
>>> case NATIVE_PARAM_USE_NATIVE_BUFFER:
>>> case NATIVE_PARAM_PRESERVE_BUFFER:
>>> case NATIVE_PARAM_MAX_SWAP_INTERVAL:
>>> @@ -283,6 +287,20 @@ wayland_surface_present(struct native_surface *nsurf,
>>> if (ctrl->preserve || ctrl->swap_interval)
>>> return FALSE;
>>>
>>> + /* force buffers to be re-created if they will be presented differently
>>> */
>>> + if (surface->premultiplied_alpha != ctrl->premultiplied_alpha) {
>>> + enum wayland_buffer_type buffer;
>>> +
>>> + for (buffer = 0; buffer < WL_BUFFER_COUNT; ++buffer) {
>>> + if (surface->buffer[buffer]) {
>>> + wl_buffer_destroy(surface->buffer[
Re: [Mesa-dev] [PATCH 4/5] st/egl: correctly return configs under wayland
2011/9/8 Chia-I Wu :
> On Thu, Sep 8, 2011 at 3:11 PM, Benjamin Franzke
> wrote:
>> First thanks for taking this on.
>>
>> There are some things I'd like to have addtionally/differently:
>>
>> Supported shm formats are exposed via a "format" event as well
>> (like the supported drm formats), so the config creation logic is the
>> same for drm and shm, and I think it can remain in native_wayland.c
>>
>> We need roundtrips to check that we get at least one supported format.
>>
>> I've attached two patches (heavily based on your last two) that do this,
>> are you ok with using them?
> That is great. Sure.
>
> I've noted two minor issues or typos, fixed by the attached patch.
> One is that param_premultiplied_alpha can be set in the common code
> and only when the display has both HAS_ARGB32 and HAS_PREMUL_ARGB32.
> The other is that we should not claim PIPE_FORMAT_B8G8R8A8_UNORM
> without HAS_ARGB32. If it looks good to you, I will commit an updated
> version of your patches.
Ok I see the point, though i can imagine compositors may want
to expose only premultilplied argb, which egl users couldnt use then,
but your right, we shouldnt expose alpha at all if one isnt available.
So I'm fine with you commiting that change.
>
>> 2011/9/8 Chia-I Wu :
>>> From: Chia-I Wu
>>>
>>> When wl_drm is avaiable and enabled, handle "format" events and return
>>> configs for the supported formats. Otherwise, assume all formats of
>>> wl_shm are supported.
>>> ---
>>> .../state_trackers/egl/wayland/native_drm.c | 70
>>> +++-
>>> .../state_trackers/egl/wayland/native_shm.c | 41 +++-
>>> .../state_trackers/egl/wayland/native_wayland.c | 28 +---
>>> .../state_trackers/egl/wayland/native_wayland.h | 4 +-
>>> 4 files changed, 113 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> index 05c32f4..facab32 100644
>>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> @@ -58,6 +58,11 @@ struct wayland_drm_display {
>>> int fd;
>>> char *device_name;
>>> boolean authenticated;
>>> +
>>> + /* supported formats */
>>> + boolean argb32;
>>> + boolean argb32_pre;
>>> + boolean xrgb32;
>>> };
>>>
>>> static INLINE struct wayland_drm_display *
>>> @@ -77,8 +82,8 @@ wayland_drm_display_destroy(struct native_display *ndpy)
>>> wl_drm_destroy(drmdpy->wl_drm);
>>> if (drmdpy->device_name)
>>> FREE(drmdpy->device_name);
>>> - if (drmdpy->base.config)
>>> - FREE(drmdpy->base.config);
>>> + if (drmdpy->base.configs)
>>> + FREE(drmdpy->base.configs);
>>> if (drmdpy->base.own_dpy)
>>> wl_display_destroy(drmdpy->base.dpy);
>>>
>>> @@ -124,6 +129,50 @@ wayland_create_drm_buffer(struct wayland_display
>>> *display,
>>> width, height, wsh.stride, format);
>>> }
>>>
>>> +static boolean
>>> +wayland_drm_display_add_configs(struct wayland_drm_display *drmdpy)
>>> +{
>>> + struct wayland_config *configs;
>>> + enum pipe_format formats[2];
>>> + int i, num_formats = 0;
>>> +
>>> + /*
>>> + * Only argb32 counts here. If we make (!argbb32 && argb32_pre) count,
>>> we
>>> + * will not be able to support the case where
>>> + * native_present_control::premultiplied_alpha is FALSE.
>>> + */
>>> + if (drmdpy->argb32)
>>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
>>> +
>>> + if (drmdpy->xrgb32)
>>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
>>> +
>>> + if (!num_formats)
>>> + return FALSE;
>>> +
>>> + configs = CALLOC(num_formats, sizeof(*configs));
>>> + if (!configs)
>>> + return FALSE;
>>> +
>>> + for (i = 0; i < num_formats; i++) {
>>> + struct native_config *nconf = &configs[i].base;
>>> +
>>> + nconf->buffer_mask =
>>> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>>> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>>> +
>>> + nconf->color_format = formats[i];
>>> +
>>> + nconf->window_bit = TRUE;
>>> + nconf->pixmap_bit = TRUE;
>>> + }
>>> +
>>> + drmdpy->base.configs = configs;
>>> + drmdpy->base.num_configs = num_formats;
>>> +
>>> + return TRUE;
>>> +}
>>> +
>>> static void
>>> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
>>> {
>>> @@ -148,7 +197,19 @@ drm_handle_device(void *data, struct wl_drm *drm,
>>> const char *device)
>>> static void
>>> drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
>>> {
>>> - /* TODO */
>>> + struct wayland_drm_display *drmdpy = data;
>>> +
>>> + switch (format) {
>>> + case WL_DRM_FORMAT_ARGB32:
>>> + drmdpy->argb32 = TRUE;
>>> + break;
>>> + case WL_DRM_FORMAT_PREMULTIPLIED_ARGB32:
>>> + drmdpy->argb32_pre = TRUE;
>>> + break;
>>> + case WL_DRM_FORMAT_XRGB32:
>>> + drmdpy->xrgb32 = TRUE;
>>> +
Re: [Mesa-dev] [PATCH 4/5] st/egl: correctly return configs under wayland
On Thu, Sep 8, 2011 at 3:41 PM, Chia-I Wu wrote:
> On Thu, Sep 8, 2011 at 3:11 PM, Benjamin Franzke
> wrote:
>> First thanks for taking this on.
>>
>> There are some things I'd like to have addtionally/differently:
>>
>> Supported shm formats are exposed via a "format" event as well
>> (like the supported drm formats), so the config creation logic is the
>> same for drm and shm, and I think it can remain in native_wayland.c
>>
>> We need roundtrips to check that we get at least one supported format.
>>
>> I've attached two patches (heavily based on your last two) that do this,
>> are you ok with using them?
> That is great. Sure.
>
> I've noted two minor issues or typos, fixed by the attached patch.
> One is that param_premultiplied_alpha can be set in the common code
> and only when the display has both HAS_ARGB32 and HAS_PREMUL_ARGB32.
After the change, we do not even need param_premultiplied_alpha.
> The other is that we should not claim PIPE_FORMAT_B8G8R8A8_UNORM
> without HAS_ARGB32. If it looks good to you, I will commit an updated
> version of your patches.
>> 2011/9/8 Chia-I Wu :
>>> From: Chia-I Wu
>>>
>>> When wl_drm is avaiable and enabled, handle "format" events and return
>>> configs for the supported formats. Otherwise, assume all formats of
>>> wl_shm are supported.
>>> ---
>>> .../state_trackers/egl/wayland/native_drm.c | 70
>>> +++-
>>> .../state_trackers/egl/wayland/native_shm.c | 41 +++-
>>> .../state_trackers/egl/wayland/native_wayland.c | 28 +---
>>> .../state_trackers/egl/wayland/native_wayland.h | 4 +-
>>> 4 files changed, 113 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> index 05c32f4..facab32 100644
>>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>>> @@ -58,6 +58,11 @@ struct wayland_drm_display {
>>> int fd;
>>> char *device_name;
>>> boolean authenticated;
>>> +
>>> + /* supported formats */
>>> + boolean argb32;
>>> + boolean argb32_pre;
>>> + boolean xrgb32;
>>> };
>>>
>>> static INLINE struct wayland_drm_display *
>>> @@ -77,8 +82,8 @@ wayland_drm_display_destroy(struct native_display *ndpy)
>>> wl_drm_destroy(drmdpy->wl_drm);
>>> if (drmdpy->device_name)
>>> FREE(drmdpy->device_name);
>>> - if (drmdpy->base.config)
>>> - FREE(drmdpy->base.config);
>>> + if (drmdpy->base.configs)
>>> + FREE(drmdpy->base.configs);
>>> if (drmdpy->base.own_dpy)
>>> wl_display_destroy(drmdpy->base.dpy);
>>>
>>> @@ -124,6 +129,50 @@ wayland_create_drm_buffer(struct wayland_display
>>> *display,
>>> width, height, wsh.stride, format);
>>> }
>>>
>>> +static boolean
>>> +wayland_drm_display_add_configs(struct wayland_drm_display *drmdpy)
>>> +{
>>> + struct wayland_config *configs;
>>> + enum pipe_format formats[2];
>>> + int i, num_formats = 0;
>>> +
>>> + /*
>>> + * Only argb32 counts here. If we make (!argbb32 && argb32_pre) count,
>>> we
>>> + * will not be able to support the case where
>>> + * native_present_control::premultiplied_alpha is FALSE.
>>> + */
>>> + if (drmdpy->argb32)
>>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
>>> +
>>> + if (drmdpy->xrgb32)
>>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
>>> +
>>> + if (!num_formats)
>>> + return FALSE;
>>> +
>>> + configs = CALLOC(num_formats, sizeof(*configs));
>>> + if (!configs)
>>> + return FALSE;
>>> +
>>> + for (i = 0; i < num_formats; i++) {
>>> + struct native_config *nconf = &configs[i].base;
>>> +
>>> + nconf->buffer_mask =
>>> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>>> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>>> +
>>> + nconf->color_format = formats[i];
>>> +
>>> + nconf->window_bit = TRUE;
>>> + nconf->pixmap_bit = TRUE;
>>> + }
>>> +
>>> + drmdpy->base.configs = configs;
>>> + drmdpy->base.num_configs = num_formats;
>>> +
>>> + return TRUE;
>>> +}
>>> +
>>> static void
>>> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
>>> {
>>> @@ -148,7 +197,19 @@ drm_handle_device(void *data, struct wl_drm *drm,
>>> const char *device)
>>> static void
>>> drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
>>> {
>>> - /* TODO */
>>> + struct wayland_drm_display *drmdpy = data;
>>> +
>>> + switch (format) {
>>> + case WL_DRM_FORMAT_ARGB32:
>>> + drmdpy->argb32 = TRUE;
>>> + break;
>>> + case WL_DRM_FORMAT_PREMULTIPLIED_ARGB32:
>>> + drmdpy->argb32_pre = TRUE;
>>> + break;
>>> + case WL_DRM_FORMAT_XRGB32:
>>> + drmdpy->xrgb32 = TRUE;
>>> + break;
>>> + }
>>> }
>>>
>>> static void
>>> @@ -191,6 +252,9 @@ wayland_drm_display_init_screen(struct native_display
>>> *ndpy)
>>> if (!drm
Re: [Mesa-dev] [PATCH 5/5] st/egl: add premultiplied alpha support to wayland
On Thu, Sep 8, 2011 at 3:13 PM, Benjamin Franzke
wrote:
> 2011/9/8 Chia-I Wu :
>> From: Chia-I Wu
>>
>> Return true for NATIVE_PARAM_PREMULTIPLIED_ALPHA when all formats with
>> alpha support premultiplied alpha. Currently, it means when argb32 and
>> argb32_pre are both supported.
>> ---
>> .../state_trackers/egl/wayland/native_drm.c | 8 ++--
>> .../state_trackers/egl/wayland/native_shm.c | 6 +-
>> .../state_trackers/egl/wayland/native_wayland.c | 18 ++
>> .../state_trackers/egl/wayland/native_wayland.h | 3 +++
>> 4 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> index facab32..e177e7c 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> @@ -114,8 +114,8 @@ wayland_create_drm_buffer(struct wayland_display
>> *display,
>>
>> switch (surface->color_format) {
>> case PIPE_FORMAT_B8G8R8A8_UNORM:
>> - /* assume premultiplied */
>> - format = WL_DRM_FORMAT_PREMULTIPLIED_ARGB32;
>> + format = (surface->premultiplied_alpha) ?
>> + WL_DRM_FORMAT_PREMULTIPLIED_ARGB32 : WL_DRM_FORMAT_ARGB32;
>> break;
>> case PIPE_FORMAT_B8G8R8X8_UNORM:
>> format = WL_DRM_FORMAT_XRGB32;
>> @@ -255,6 +255,10 @@ wayland_drm_display_init_screen(struct native_display
>> *ndpy)
>> if (!wayland_drm_display_add_configs(drmdpy))
>> return FALSE;
>>
>> + /* check that premultiplied alpha is supported for all formats with
>> alpha */
>> + if (!drmdpy->argb32 || drmdpy->argb32_pre)
>> + drmdpy->base.param_premultiplied_alpha = TRUE;
>
> Why enable premultiplied alpha if argb32 is not exposed?
> What isnt covered with just: "if (drmdpy->argb32_pre)"?.
Yes, it is simpler. What I intended to do is to enable pre-multiplied
alpha when all formats that have alpha also support pre-multiplied
alpha. In the case there is no format that has alpha
(!drmdpy->argb32), pre-multiplied alpha can be enabled.
>> +
>> drmdpy->base.base.screen =
>> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
>> NULL, drmdpy->fd);
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c
>> b/src/gallium/state_trackers/egl/wayland/native_shm.c
>> index 5882e74..e2d2437 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_shm.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
>> @@ -95,7 +95,8 @@ wayland_create_shm_buffer(struct wayland_display *display,
>>
>> switch (surface->color_format) {
>> case PIPE_FORMAT_B8G8R8A8_UNORM:
>> - format = WL_SHM_FORMAT_PREMULTIPLIED_ARGB32;
>> + format = (surface->premultiplied_alpha) ?
>> + WL_SHM_FORMAT_PREMULTIPLIED_ARGB32 : WL_SHM_FORMAT_ARGB32;
>> break;
>> case PIPE_FORMAT_B8G8R8X8_UNORM:
>> format = WL_SHM_FORMAT_XRGB32;
>> @@ -165,6 +166,9 @@ wayland_shm_display_init_screen(struct native_display
>> *ndpy)
>> if (!wayland_shm_display_add_configs(shmdpy))
>> return FALSE;
>>
>> + /* assume all formats are supported */
>> + shmdpy->base.param_premultiplied_alpha = TRUE;
>> +
>> winsys = wayland_create_sw_winsys(shmdpy->base.dpy);
>> if (!winsys)
>> return FALSE;
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> index 14cc908..b2dab8f 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c
>> @@ -60,9 +60,13 @@ static int
>> wayland_display_get_param(struct native_display *ndpy,
>> enum native_param_type param)
>> {
>> + struct wayland_display *display = wayland_display(ndpy);
>> int val;
>>
>> switch (param) {
>> + case NATIVE_PARAM_PREMULTIPLIED_ALPHA:
>> + val = display->param_premultiplied_alpha;
>> + break;
>> case NATIVE_PARAM_USE_NATIVE_BUFFER:
>> case NATIVE_PARAM_PRESERVE_BUFFER:
>> case NATIVE_PARAM_MAX_SWAP_INTERVAL:
>> @@ -283,6 +287,20 @@ wayland_surface_present(struct native_surface *nsurf,
>> if (ctrl->preserve || ctrl->swap_interval)
>> return FALSE;
>>
>> + /* force buffers to be re-created if they will be presented differently
>> */
>> + if (surface->premultiplied_alpha != ctrl->premultiplied_alpha) {
>> + enum wayland_buffer_type buffer;
>> +
>> + for (buffer = 0; buffer < WL_BUFFER_COUNT; ++buffer) {
>> + if (surface->buffer[buffer]) {
>> + wl_buffer_destroy(surface->buffer[buffer]);
>> + surface->buffer[buffer] = NULL;
>> + }
>> + }
>> +
>> + surface->premultiplied_alpha = ctrl->premultiplied_alpha;
>> + }
>> +
>> switch (ctrl->natt) {
>> case NATIVE_ATTACHMENT_FRONT_LEFT:
>> ret = TRUE;
>> diff --git a/src/gallium/stat
Re: [Mesa-dev] [PATCH 4/5] st/egl: correctly return configs under wayland
On Thu, Sep 8, 2011 at 3:11 PM, Benjamin Franzke
wrote:
> First thanks for taking this on.
>
> There are some things I'd like to have addtionally/differently:
>
> Supported shm formats are exposed via a "format" event as well
> (like the supported drm formats), so the config creation logic is the
> same for drm and shm, and I think it can remain in native_wayland.c
>
> We need roundtrips to check that we get at least one supported format.
>
> I've attached two patches (heavily based on your last two) that do this,
> are you ok with using them?
That is great. Sure.
I've noted two minor issues or typos, fixed by the attached patch.
One is that param_premultiplied_alpha can be set in the common code
and only when the display has both HAS_ARGB32 and HAS_PREMUL_ARGB32.
The other is that we should not claim PIPE_FORMAT_B8G8R8A8_UNORM
without HAS_ARGB32. If it looks good to you, I will commit an updated
version of your patches.
> 2011/9/8 Chia-I Wu :
>> From: Chia-I Wu
>>
>> When wl_drm is avaiable and enabled, handle "format" events and return
>> configs for the supported formats. Otherwise, assume all formats of
>> wl_shm are supported.
>> ---
>> .../state_trackers/egl/wayland/native_drm.c | 70
>> +++-
>> .../state_trackers/egl/wayland/native_shm.c | 41 +++-
>> .../state_trackers/egl/wayland/native_wayland.c | 28 +---
>> .../state_trackers/egl/wayland/native_wayland.h | 4 +-
>> 4 files changed, 113 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> index 05c32f4..facab32 100644
>> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
>> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
>> @@ -58,6 +58,11 @@ struct wayland_drm_display {
>> int fd;
>> char *device_name;
>> boolean authenticated;
>> +
>> + /* supported formats */
>> + boolean argb32;
>> + boolean argb32_pre;
>> + boolean xrgb32;
>> };
>>
>> static INLINE struct wayland_drm_display *
>> @@ -77,8 +82,8 @@ wayland_drm_display_destroy(struct native_display *ndpy)
>> wl_drm_destroy(drmdpy->wl_drm);
>> if (drmdpy->device_name)
>> FREE(drmdpy->device_name);
>> - if (drmdpy->base.config)
>> - FREE(drmdpy->base.config);
>> + if (drmdpy->base.configs)
>> + FREE(drmdpy->base.configs);
>> if (drmdpy->base.own_dpy)
>> wl_display_destroy(drmdpy->base.dpy);
>>
>> @@ -124,6 +129,50 @@ wayland_create_drm_buffer(struct wayland_display
>> *display,
>> width, height, wsh.stride, format);
>> }
>>
>> +static boolean
>> +wayland_drm_display_add_configs(struct wayland_drm_display *drmdpy)
>> +{
>> + struct wayland_config *configs;
>> + enum pipe_format formats[2];
>> + int i, num_formats = 0;
>> +
>> + /*
>> + * Only argb32 counts here. If we make (!argbb32 && argb32_pre) count,
>> we
>> + * will not be able to support the case where
>> + * native_present_control::premultiplied_alpha is FALSE.
>> + */
>> + if (drmdpy->argb32)
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
>> +
>> + if (drmdpy->xrgb32)
>> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
>> +
>> + if (!num_formats)
>> + return FALSE;
>> +
>> + configs = CALLOC(num_formats, sizeof(*configs));
>> + if (!configs)
>> + return FALSE;
>> +
>> + for (i = 0; i < num_formats; i++) {
>> + struct native_config *nconf = &configs[i].base;
>> +
>> + nconf->buffer_mask =
>> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
>> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
>> +
>> + nconf->color_format = formats[i];
>> +
>> + nconf->window_bit = TRUE;
>> + nconf->pixmap_bit = TRUE;
>> + }
>> +
>> + drmdpy->base.configs = configs;
>> + drmdpy->base.num_configs = num_formats;
>> +
>> + return TRUE;
>> +}
>> +
>> static void
>> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
>> {
>> @@ -148,7 +197,19 @@ drm_handle_device(void *data, struct wl_drm *drm, const
>> char *device)
>> static void
>> drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
>> {
>> - /* TODO */
>> + struct wayland_drm_display *drmdpy = data;
>> +
>> + switch (format) {
>> + case WL_DRM_FORMAT_ARGB32:
>> + drmdpy->argb32 = TRUE;
>> + break;
>> + case WL_DRM_FORMAT_PREMULTIPLIED_ARGB32:
>> + drmdpy->argb32_pre = TRUE;
>> + break;
>> + case WL_DRM_FORMAT_XRGB32:
>> + drmdpy->xrgb32 = TRUE;
>> + break;
>> + }
>> }
>>
>> static void
>> @@ -191,6 +252,9 @@ wayland_drm_display_init_screen(struct native_display
>> *ndpy)
>> if (!drmdpy->authenticated)
>> return FALSE;
>>
>> + if (!wayland_drm_display_add_configs(drmdpy))
>> + return FALSE;
>> +
>> drmdpy->base.base.screen =
>> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
>>
Re: [Mesa-dev] [PATCH 5/5] st/egl: add premultiplied alpha support to wayland
2011/9/8 Chia-I Wu :
> From: Chia-I Wu
>
> Return true for NATIVE_PARAM_PREMULTIPLIED_ALPHA when all formats with
> alpha support premultiplied alpha. Currently, it means when argb32 and
> argb32_pre are both supported.
> ---
> .../state_trackers/egl/wayland/native_drm.c | 8 ++--
> .../state_trackers/egl/wayland/native_shm.c | 6 +-
> .../state_trackers/egl/wayland/native_wayland.c | 18 ++
> .../state_trackers/egl/wayland/native_wayland.h | 3 +++
> 4 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
> b/src/gallium/state_trackers/egl/wayland/native_drm.c
> index facab32..e177e7c 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
> @@ -114,8 +114,8 @@ wayland_create_drm_buffer(struct wayland_display *display,
>
> switch (surface->color_format) {
> case PIPE_FORMAT_B8G8R8A8_UNORM:
> - /* assume premultiplied */
> - format = WL_DRM_FORMAT_PREMULTIPLIED_ARGB32;
> + format = (surface->premultiplied_alpha) ?
> + WL_DRM_FORMAT_PREMULTIPLIED_ARGB32 : WL_DRM_FORMAT_ARGB32;
> break;
> case PIPE_FORMAT_B8G8R8X8_UNORM:
> format = WL_DRM_FORMAT_XRGB32;
> @@ -255,6 +255,10 @@ wayland_drm_display_init_screen(struct native_display
> *ndpy)
> if (!wayland_drm_display_add_configs(drmdpy))
> return FALSE;
>
> + /* check that premultiplied alpha is supported for all formats with alpha
> */
> + if (!drmdpy->argb32 || drmdpy->argb32_pre)
> + drmdpy->base.param_premultiplied_alpha = TRUE;
Why enable premultiplied alpha if argb32 is not exposed?
What isnt covered with just: "if (drmdpy->argb32_pre)"?.
> +
> drmdpy->base.base.screen =
> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
> NULL, drmdpy->fd);
> diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c
> b/src/gallium/state_trackers/egl/wayland/native_shm.c
> index 5882e74..e2d2437 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_shm.c
> +++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
> @@ -95,7 +95,8 @@ wayland_create_shm_buffer(struct wayland_display *display,
>
> switch (surface->color_format) {
> case PIPE_FORMAT_B8G8R8A8_UNORM:
> - format = WL_SHM_FORMAT_PREMULTIPLIED_ARGB32;
> + format = (surface->premultiplied_alpha) ?
> + WL_SHM_FORMAT_PREMULTIPLIED_ARGB32 : WL_SHM_FORMAT_ARGB32;
> break;
> case PIPE_FORMAT_B8G8R8X8_UNORM:
> format = WL_SHM_FORMAT_XRGB32;
> @@ -165,6 +166,9 @@ wayland_shm_display_init_screen(struct native_display
> *ndpy)
> if (!wayland_shm_display_add_configs(shmdpy))
> return FALSE;
>
> + /* assume all formats are supported */
> + shmdpy->base.param_premultiplied_alpha = TRUE;
> +
> winsys = wayland_create_sw_winsys(shmdpy->base.dpy);
> if (!winsys)
> return FALSE;
> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c
> b/src/gallium/state_trackers/egl/wayland/native_wayland.c
> index 14cc908..b2dab8f 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.c
> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c
> @@ -60,9 +60,13 @@ static int
> wayland_display_get_param(struct native_display *ndpy,
> enum native_param_type param)
> {
> + struct wayland_display *display = wayland_display(ndpy);
> int val;
>
> switch (param) {
> + case NATIVE_PARAM_PREMULTIPLIED_ALPHA:
> + val = display->param_premultiplied_alpha;
> + break;
> case NATIVE_PARAM_USE_NATIVE_BUFFER:
> case NATIVE_PARAM_PRESERVE_BUFFER:
> case NATIVE_PARAM_MAX_SWAP_INTERVAL:
> @@ -283,6 +287,20 @@ wayland_surface_present(struct native_surface *nsurf,
> if (ctrl->preserve || ctrl->swap_interval)
> return FALSE;
>
> + /* force buffers to be re-created if they will be presented differently */
> + if (surface->premultiplied_alpha != ctrl->premultiplied_alpha) {
> + enum wayland_buffer_type buffer;
> +
> + for (buffer = 0; buffer < WL_BUFFER_COUNT; ++buffer) {
> + if (surface->buffer[buffer]) {
> + wl_buffer_destroy(surface->buffer[buffer]);
> + surface->buffer[buffer] = NULL;
> + }
> + }
> +
> + surface->premultiplied_alpha = ctrl->premultiplied_alpha;
> + }
> +
> switch (ctrl->natt) {
> case NATIVE_ATTACHMENT_FRONT_LEFT:
> ret = TRUE;
> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.h
> b/src/gallium/state_trackers/egl/wayland/native_wayland.h
> index 93e670b..6cf98a8 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.h
> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.h
> @@ -44,6 +44,8 @@ struct wayland_display {
>
> struct wayland_config *configs;
> int num_configs;
> + /* true if all formats with alpha support premultiplied
Re: [Mesa-dev] [PATCH 4/5] st/egl: correctly return configs under wayland
First thanks for taking this on.
There are some things I'd like to have addtionally/differently:
Supported shm formats are exposed via a "format" event as well
(like the supported drm formats), so the config creation logic is the
same for drm and shm, and I think it can remain in native_wayland.c
We need roundtrips to check that we get at least one supported format.
I've attached two patches (heavily based on your last two) that do this,
are you ok with using them?
2011/9/8 Chia-I Wu :
> From: Chia-I Wu
>
> When wl_drm is avaiable and enabled, handle "format" events and return
> configs for the supported formats. Otherwise, assume all formats of
> wl_shm are supported.
> ---
> .../state_trackers/egl/wayland/native_drm.c | 70
> +++-
> .../state_trackers/egl/wayland/native_shm.c | 41 +++-
> .../state_trackers/egl/wayland/native_wayland.c | 28 +---
> .../state_trackers/egl/wayland/native_wayland.h | 4 +-
> 4 files changed, 113 insertions(+), 30 deletions(-)
>
> diff --git a/src/gallium/state_trackers/egl/wayland/native_drm.c
> b/src/gallium/state_trackers/egl/wayland/native_drm.c
> index 05c32f4..facab32 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_drm.c
> +++ b/src/gallium/state_trackers/egl/wayland/native_drm.c
> @@ -58,6 +58,11 @@ struct wayland_drm_display {
> int fd;
> char *device_name;
> boolean authenticated;
> +
> + /* supported formats */
> + boolean argb32;
> + boolean argb32_pre;
> + boolean xrgb32;
> };
>
> static INLINE struct wayland_drm_display *
> @@ -77,8 +82,8 @@ wayland_drm_display_destroy(struct native_display *ndpy)
> wl_drm_destroy(drmdpy->wl_drm);
> if (drmdpy->device_name)
> FREE(drmdpy->device_name);
> - if (drmdpy->base.config)
> - FREE(drmdpy->base.config);
> + if (drmdpy->base.configs)
> + FREE(drmdpy->base.configs);
> if (drmdpy->base.own_dpy)
> wl_display_destroy(drmdpy->base.dpy);
>
> @@ -124,6 +129,50 @@ wayland_create_drm_buffer(struct wayland_display
> *display,
> width, height, wsh.stride, format);
> }
>
> +static boolean
> +wayland_drm_display_add_configs(struct wayland_drm_display *drmdpy)
> +{
> + struct wayland_config *configs;
> + enum pipe_format formats[2];
> + int i, num_formats = 0;
> +
> + /*
> + * Only argb32 counts here. If we make (!argbb32 && argb32_pre) count, we
> + * will not be able to support the case where
> + * native_present_control::premultiplied_alpha is FALSE.
> + */
> + if (drmdpy->argb32)
> + formats[num_formats++] = PIPE_FORMAT_B8G8R8A8_UNORM;
> +
> + if (drmdpy->xrgb32)
> + formats[num_formats++] = PIPE_FORMAT_B8G8R8X8_UNORM;
> +
> + if (!num_formats)
> + return FALSE;
> +
> + configs = CALLOC(num_formats, sizeof(*configs));
> + if (!configs)
> + return FALSE;
> +
> + for (i = 0; i < num_formats; i++) {
> + struct native_config *nconf = &configs[i].base;
> +
> + nconf->buffer_mask =
> + (1 << NATIVE_ATTACHMENT_FRONT_LEFT) |
> + (1 << NATIVE_ATTACHMENT_BACK_LEFT);
> +
> + nconf->color_format = formats[i];
> +
> + nconf->window_bit = TRUE;
> + nconf->pixmap_bit = TRUE;
> + }
> +
> + drmdpy->base.configs = configs;
> + drmdpy->base.num_configs = num_formats;
> +
> + return TRUE;
> +}
> +
> static void
> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
> {
> @@ -148,7 +197,19 @@ drm_handle_device(void *data, struct wl_drm *drm, const
> char *device)
> static void
> drm_handle_format(void *data, struct wl_drm *drm, uint32_t format)
> {
> - /* TODO */
> + struct wayland_drm_display *drmdpy = data;
> +
> + switch (format) {
> + case WL_DRM_FORMAT_ARGB32:
> + drmdpy->argb32 = TRUE;
> + break;
> + case WL_DRM_FORMAT_PREMULTIPLIED_ARGB32:
> + drmdpy->argb32_pre = TRUE;
> + break;
> + case WL_DRM_FORMAT_XRGB32:
> + drmdpy->xrgb32 = TRUE;
> + break;
> + }
> }
>
> static void
> @@ -191,6 +252,9 @@ wayland_drm_display_init_screen(struct native_display
> *ndpy)
> if (!drmdpy->authenticated)
> return FALSE;
>
> + if (!wayland_drm_display_add_configs(drmdpy))
> + return FALSE;
> +
> drmdpy->base.base.screen =
> drmdpy->event_handler->new_drm_screen(&drmdpy->base.base,
> NULL, drmdpy->fd);
> diff --git a/src/gallium/state_trackers/egl/wayland/native_shm.c
> b/src/gallium/state_trackers/egl/wayland/native_shm.c
> index 598df9f..5882e74 100644
> --- a/src/gallium/state_trackers/egl/wayland/native_shm.c
> +++ b/src/gallium/state_trackers/egl/wayland/native_shm.c
> @@ -63,8 +63,8 @@ wayland_shm_display_destroy(struct native_display *ndpy)
> {
> struct wayland_shm_display *shmdpy = wayland_shm_display(ndpy);
>
> - if (shmdpy->base.config)
> - FREE(shmdpy->base.config);
> + if (shmdpy->base.configs)
> + FREE(shmdpy->base.configs);
>
Re: [Mesa-dev] State tracker pipe_surface woes...
2011/9/7 Roland Scheidegger : > That said I'm not really sure why pipe_sampler_view and pipe_surface > actually have a context pointer in them, since they are only supposed to > be used with the context in which they were created I think those > shouldn't actually exist - surface_destroy and sampler_view_destroy will > have context as a parameter, and if they aren't shared between contexts > it's pointless to store the context pointer. Might be a relic from when > those structs were created/destroyed using screen functions... The ctx pointer is there because of pipe_surface_reference and pipe_sampler_view_reference. When the refcount becomes 0, the corresponding pipe...reference function uses the ctx pointer to do ctx->...destroy(ctx, ... So that we can destroy objects just by: pipe_surface_reference(&surface, NULL); This ctx pointer also ensures that the destroy function is called with the right context. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
