Re: [Mesa-dev] [PATCH] R600/SI: Add pattern for fp_to_uint
On Die, 2013-07-30 at 03:45 +0200, Marek Olšák wrote: This fixes the F2U opcode for the Mesa driver. Signed-off-by: Marek Olšák marek.ol...@amd.com Reviewed-by: Michel Dänzer michel.daen...@amd.com Do you have LLVM SVN write access? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] glsl: Switch from the deprecated YYLEX_PARAM to %lex-param.
On Mon, Jul 29, 2013 at 8:44 PM, Nikita Malyavin nikitamalya...@gmail.com wrote: Kenneth, maybe it's better to mark _mesa_glsl_lex as inline? The compiler will figure it out. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] R600/SI: Add pattern for fp_to_uint
No, I don't. This is a git patch. Marek On Tue, Jul 30, 2013 at 7:53 AM, Michel Dänzer mic...@daenzer.net wrote: On Die, 2013-07-30 at 03:45 +0200, Marek Olšák wrote: This fixes the F2U opcode for the Mesa driver. Signed-off-by: Marek Olšák marek.ol...@amd.com Reviewed-by: Michel Dänzer michel.daen...@amd.com Do you have LLVM SVN write access? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 58734] Add support for GL_EXT_bindable_uniform - Dungeon Defenders fails to launch crash
https://bugs.freedesktop.org/show_bug.cgi?id=58734 ruthubuntu ruthubu...@gmx.cn changed: What|Removed |Added CC||ruthubu...@gmx.cn -- 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 67516] glTexStorage*() functions don't work properly with proxy textures
https://bugs.freedesktop.org/show_bug.cgi?id=67516 --- Comment #4 from Brian Paul bri...@vmware.com --- The patches look good to me. Reviewed-by: Brian Paul bri...@vmware.com Do you need me to commit them for you? -- 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 67516] glTexStorage*() functions don't work properly with proxy textures
https://bugs.freedesktop.org/show_bug.cgi?id=67516 --- Comment #5 from Mikko Juola mik...@gmail.com --- Yeah, I think so. This is the first time I've ever submitted any patches to mesa. -- 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] R600/SI: Add pattern for fp_to_uint
On Tue, Jul 30, 2013 at 03:45:13AM +0200, Marek Olšák wrote: This fixes the F2U opcode for the Mesa driver. Signed-off-by: Marek Olšák marek.ol...@amd.com Hi Marek, You will need to include a lit test with this patch. lit tests are located in test/CodeGen/R600. You can reuse the existing fp_to_uint.ll test and just add CHECK lines for SI. I just pushed a patch: R600/SI: Expand vector fp - int conversions which prevents that test from crashing on SI, and you can use that as an example for how to update a test. For commit access, you will need to email Chris, see http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access for instructions. I use git-svn for working with the LLVM repository and it works pretty well. There are good instructions for how to set it up here: http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-git-svn In order to reduce the LLVM build time, I only build the R600 and X86 targets (mesa requires the X86 target even if you aren't building llvmpipe), and I use ccache. The other thing you should do is make sure that you don't have a debug version of clang installed on your system, because LLVM will use clang by default and the debug versions are really slow. The solution to this is to force LLVM to build with gcc by adding CC=gcc CXX=g++ to your configure script or by installing a release build of clang. Also, if you try to run piglit with a debug version of LLVM, it will take forever. I always install a release version of llvm for testing and running piglit, and if I need to debug an individual test, I build (but don't install) a debug version of LLVM using: make ENABLE_OPTIMIZED=0 and then use LD_PRELOAD=llvm/Debug+Asserts/lib/libLLVM-3.4svn.so some_app for testing. Hope this helps, let me know if you have any other questions. -Tom --- lib/Target/R600/SIInstructions.td | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td index 0d50c5d..c392238 100644 --- a/lib/Target/R600/SIInstructions.td +++ b/lib/Target/R600/SIInstructions.td @@ -605,7 +605,9 @@ defm V_CVT_F32_I32 : VOP1_32 0x0005, V_CVT_F32_I32, defm V_CVT_F32_U32 : VOP1_32 0x0006, V_CVT_F32_U32, [(set f32:$dst, (uint_to_fp i32:$src0))] ; -defm V_CVT_U32_F32 : VOP1_32 0x0007, V_CVT_U32_F32, []; +defm V_CVT_U32_F32 : VOP1_32 0x0007, V_CVT_U32_F32, + [(set i32:$dst, (fp_to_uint f32:$src0))] +; defm V_CVT_I32_F32 : VOP1_32 0x0008, V_CVT_I32_F32, [(set i32:$dst, (fp_to_sint f32:$src0))] ; -- 1.8.1.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Added missing address space checking of kernel parameters
On Wed, Jul 24, 2013 at 09:29:49AM -0400, Jonathan Charest wrote:
Here is an updated patch with no line wrapping and respecting 80-column limit
(for my changes).
Hi,
I've pushed a modified version of this patch that uses global arguments
for constant buffers so it doesn't break r600g, and I've pushed your
r600g lds size fix too. Thanks!
-Tom
---
.../state_trackers/clover/llvm/invocation.cpp | 34
+++---
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index dae61f7..3846a6e 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -26,6 +26,7 @@
#include clang/Frontend/TextDiagnosticBuffer.h
#include clang/Frontend/TextDiagnosticPrinter.h
#include clang/CodeGen/CodeGenAction.h
+#include clang/Basic/TargetInfo.h
#include llvm/Bitcode/BitstreamWriter.h
#include llvm/Bitcode/ReaderWriter.h
#include llvm/Linker.h
@@ -113,7 +114,7 @@ namespace {
llvm::Module *
compile(const std::string source, const std::string name,
const std::string triple, const std::string processor,
- const std::string opts) {
+ const std::string opts, clang::LangAS::Map address_spaces) {
clang::CompilerInstance c;
clang::CompilerInvocation invocation;
@@ -204,6 +205,10 @@ namespace {
if (!c.ExecuteAction(act))
throw build_error(log);
+ // Get address spaces map to be able to find kernel argument address
space
+ memcpy(address_spaces, c.getTarget().getAddressSpaceMap(),
+
sizeof(address_spaces));
+
return act.takeModule();
}
@@ -282,7 +287,8 @@ namespace {
module
build_module_llvm(llvm::Module *mod,
- const std::vectorllvm::Function * kernels) {
+ const std::vectorllvm::Function * kernels,
+ clang::LangAS::Map address_spaces) {
module m;
struct pipe_llvm_program_header header;
@@ -318,14 +324,18 @@ namespace {
}
if (arg_type-isPointerTy()) {
- // XXX: Figure out LLVM-OpenCL address space mappings for
each
- // target. I think we need to ask clang what these are. For
now,
- // pretend everything is in the global address space.
unsigned address_space =
llvm::castllvm::PointerType(arg_type)-getAddressSpace();
- switch (address_space) {
- default:
-
args.push_back(module::argument(module::argument::global, arg_size));
- break;
+ if (address_space ==
address_spaces[clang::LangAS::opencl_local
+ -
clang::LangAS::Offset]) {
+ args.push_back(module::argument(module::argument::local,
+
arg_size));
+ } else if (address_space == address_spaces[
+ clang::LangAS::opencl_constant -
clang::LangAS::Offset]) {
+
args.push_back(module::argument(module::argument::constant,
+
arg_size));
+ } else {
+ args.push_back(module::argument(module::argument::global,
+
arg_size));
}
} else {
args.push_back(module::argument(module::argument::scalar,
arg_size));
@@ -358,10 +368,12 @@ clover::compile_program_llvm(const compat::string
source,
std::string processor(target.begin(), 0, processor_str_len);
std::string triple(target.begin(), processor_str_len + 1,
target.size() - processor_str_len - 1);
+ clang::LangAS::Map address_spaces;
// The input file name must have the .cl extension in order for the
// CompilerInvocation class to recognize it as an OpenCL source file.
- llvm::Module *mod = compile(source, input.cl, triple, processor, opts);
+ llvm::Module *mod = compile(source, input.cl, triple, processor, opts,
+
address_spaces);
find_kernels(mod, kernels);
@@ -374,6 +386,6 @@ clover::compile_program_llvm(const compat::string source,
assert(0);
return module();
default:
- return build_module_llvm(mod, kernels);
+ return build_module_llvm(mod, kernels, address_spaces);
}
}
--
1.8.3.3
On 2013-07-24 03:58, Francisco Jerez wrote:
Tom Stellard t...@stellard.net writes:
On Mon, Jul 22, 2013 at 09:24:12AM -0400, Jonathan Charest wrote:
[Mesa-dev] [Bug 58734] Add support for GL_EXT_bindable_uniform - Dungeon Defenders fails to launch crash
https://bugs.freedesktop.org/show_bug.cgi?id=58734 Kenneth Graunke kenn...@whitecape.org changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #4 from Kenneth Graunke kenn...@whitecape.org --- This should've been closed a long time ago :) Ryan Gordon fixed Dungeon Defenders clear back in March, if not earlier. The latest Steam builds work fine, as do the Humble Bundle builds. -- 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] R600/SI: Add pattern for fp_to_uint
On Die, 2013-07-30 at 07:47 -0700, Tom Stellard wrote: You will need to include a lit test with this patch. Ah yes, this occurred to me after my review. :) Also, if you try to run piglit with a debug version of LLVM, it will take forever. I always install a release version of llvm for testing and running piglit, and if I need to debug an individual test, I build (but don't install) a debug version of LLVM using: make ENABLE_OPTIMIZED=0 and then use LD_PRELOAD=llvm/Debug+Asserts/lib/libLLVM-3.4svn.so some_app for testing. FWIW, I'm building LLVM with '--enable-optimized --with-optimize-option=-O2 --enable-assertions --enable-debug-runtime --enable-debug-symbols', which has been fast enough for piglit (less than 10 minutes for quick-driver.tests) and good enough for debugging for me. YMMV. Also, building LLVM with --enable-shared will save a significant amount of time for linking in Mesa (and possibly LLVM as well). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/34] mesa/st: Add VARYING_SLOT_TEX[1-7] to st_translate_geometry_program().
On 29.07.2013 08:03, Paul Berry wrote: From: Bryan Cain bryanca...@gmail.com v2 (Paul Berry stereotype...@gmail.com: Split out to separate patch (previously this was part of glsl: add builtins for geometry shaders.) --- src/mesa/state_tracker/st_program.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c index 60cc37c..211b879 100644 --- a/src/mesa/state_tracker/st_program.c +++ b/src/mesa/state_tracker/st_program.c @@ -911,6 +911,13 @@ st_translate_geometry_program(struct st_context *st, stgp-input_semantic_index[slot] = 0; break; case VARYING_SLOT_TEX0: + case VARYING_SLOT_TEX1: + case VARYING_SLOT_TEX2: + case VARYING_SLOT_TEX3: + case VARYING_SLOT_TEX4: + case VARYING_SLOT_TEX5: + case VARYING_SLOT_TEX6: + case VARYING_SLOT_TEX7: stgp-input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC; stgp-input_semantic_index[slot] = num_generic++; break; This doesn't work, first because the semantic index shouldn't depend on which varyings are present, and second because TEX is required to use TGSI_SEMANTIC_TEXCOORD if the driver has PIPE_CAP_TGSI_TEXCOORD. Please see st_prepare_vertex_program. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] R600/SI: Add pattern for fp_to_uint
On Tue, Jul 30, 2013 at 05:54:58PM +0200, Michel Dänzer wrote: On Die, 2013-07-30 at 07:47 -0700, Tom Stellard wrote: You will need to include a lit test with this patch. Ah yes, this occurred to me after my review. :) Also, if you try to run piglit with a debug version of LLVM, it will take forever. I always install a release version of llvm for testing and running piglit, and if I need to debug an individual test, I build (but don't install) a debug version of LLVM using: make ENABLE_OPTIMIZED=0 and then use LD_PRELOAD=llvm/Debug+Asserts/lib/libLLVM-3.4svn.so some_app for testing. FWIW, I'm building LLVM with '--enable-optimized --with-optimize-option=-O2 --enable-assertions --enable-debug-runtime --enable-debug-symbols', which has been fast enough for piglit (less than 10 minutes for quick-driver.tests) and good enough for debugging for me. YMMV. Oh nice, I wasn't aware of those options. I'll have to try that. -Tom Also, building LLVM with --enable-shared will save a significant amount of time for linking in Mesa (and possibly LLVM as well). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] gallium: clarify shift behavior with shift count = 32
From: Roland Scheidegger srol...@vmware.com Previously, nothing was said what happens with shift counts exceeding bit width of the values to shift. In theory 3 behaviors are possible: 1) undefined (classic c definition) 2) just shift out all bits (so result is zero, or -1 potentially for ashr) 3) mask the shift count to bit width - 1 API's either require 3) or are ok with 1). In particular, GLSL (as well as a couple uninteresting legacy GL extensions) is happy with undefined, whereas both OpenCL and d3d10 require 3). Consequently, most hw also implements 3). So, for simplicity we just specify that 3) is required rather than saying undefined and then needing state trackers to work around it. Also while here specify shift count as a vector, not scalar. As far as I can tell this was a doc bug, neither state trackers nor drivers used scalar shift count. --- src/gallium/docs/source/tgsi.rst | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/gallium/docs/source/tgsi.rst b/src/gallium/docs/source/tgsi.rst index 0557ce0..8506b7e 100644 --- a/src/gallium/docs/source/tgsi.rst +++ b/src/gallium/docs/source/tgsi.rst @@ -1254,41 +1254,47 @@ Support for these opcodes indicated by PIPE_SHADER_CAP_INTEGERS (all of them?) .. opcode:: SHL - Shift Left + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x src1.x + dst.x = src0.x (0x1f src1.x) - dst.y = src0.y src1.x + dst.y = src0.y (0x1f src1.y) - dst.z = src0.z src1.x + dst.z = src0.z (0x1f src1.z) - dst.w = src0.w src1.x + dst.w = src0.w (0x1f src1.w) .. opcode:: ISHR - Arithmetic Shift Right (of Signed Integer) + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x src1.x + dst.x = src0.x (0x1f src1.x) - dst.y = src0.y src1.x + dst.y = src0.y (0x1f src1.y) - dst.z = src0.z src1.x + dst.z = src0.z (0x1f src1.z) - dst.w = src0.w src1.x + dst.w = src0.w (0x1f src1.w) .. opcode:: USHR - Logical Shift Right + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x (unsigned) src1.x + dst.x = src0.x (unsigned) (0x1f src1.x) - dst.y = src0.y (unsigned) src1.x + dst.y = src0.y (unsigned) (0x1f src1.y) - dst.z = src0.z (unsigned) src1.x + dst.z = src0.z (unsigned) (0x1f src1.z) - dst.w = src0.w (unsigned) src1.x + dst.w = src0.w (unsigned) (0x1f src1.w) .. opcode:: UCMP - Integer Conditional Move -- 1.7.9.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] gallivm: obey clarified shift behavior
From: Roland Scheidegger srol...@vmware.com
llvm shifts are undefined for shift counts exceeding (or matching) bit width,
so need to apply a mask.
NOTE: there's internal callers using this which guarantee the shift count
is smaller than the type width. However, all of these use constant shift
counts hence the additional AND will get dropped by llvm (hopefully, some
quick observation at least showed it working). There is also code using shifts
where we know the shift count must be smaller than type width but llvm does
not, however these all seem to be using LLVMBuildLShr/AShr/Shl directly (like
lp_build_minify for mip level minification), hence at this point don't
introduce additional lp_build_shl/shr functions which skip the masking.
---
src/gallium/auxiliary/gallivm/lp_bld_bitarit.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
b/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
index 97ae162..7cae09d 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
@@ -168,6 +168,7 @@ lp_build_not(struct lp_build_context *bld, LLVMValueRef a)
/**
* Shift left.
+ * The shift count is masked to type width - 1.
*/
LLVMValueRef
lp_build_shl(struct lp_build_context *bld, LLVMValueRef a, LLVMValueRef b)
@@ -181,6 +182,9 @@ lp_build_shl(struct lp_build_context *bld, LLVMValueRef a,
LLVMValueRef b)
assert(lp_check_value(type, a));
assert(lp_check_value(type, b));
+ b = LLVMBuildAnd(builder, b,
+lp_build_const_vec(bld-gallivm, type, type.width - 1),
);
+
res = LLVMBuildShl(builder, a, b, );
return res;
@@ -189,6 +193,7 @@ lp_build_shl(struct lp_build_context *bld, LLVMValueRef a,
LLVMValueRef b)
/**
* Shift right.
+ * The shift count is masked to type width - 1.
*/
LLVMValueRef
lp_build_shr(struct lp_build_context *bld, LLVMValueRef a, LLVMValueRef b)
@@ -202,6 +207,9 @@ lp_build_shr(struct lp_build_context *bld, LLVMValueRef a,
LLVMValueRef b)
assert(lp_check_value(type, a));
assert(lp_check_value(type, b));
+ b = LLVMBuildAnd(builder, b,
+lp_build_const_vec(bld-gallivm, type, type.width - 1),
);
+
if (type.sign) {
res = LLVMBuildAShr(builder, a, b, );
} else {
@@ -214,23 +222,25 @@ lp_build_shr(struct lp_build_context *bld, LLVMValueRef
a, LLVMValueRef b)
/**
* Shift left with immediate.
+ * The immediate shift count must be smaller than the type width.
*/
LLVMValueRef
lp_build_shl_imm(struct lp_build_context *bld, LLVMValueRef a, unsigned imm)
{
LLVMValueRef b = lp_build_const_int_vec(bld-gallivm, bld-type, imm);
- assert(imm = bld-type.width);
+ assert(imm bld-type.width);
return lp_build_shl(bld, a, b);
}
/**
* Shift right with immediate.
+ * The immediate shift count must be smaller than the type width.
*/
LLVMValueRef
lp_build_shr_imm(struct lp_build_context *bld, LLVMValueRef a, unsigned imm)
{
LLVMValueRef b = lp_build_const_int_vec(bld-gallivm, bld-type, imm);
- assert(imm = bld-type.width);
+ assert(imm bld-type.width);
return lp_build_shr(bld, a, b);
}
--
1.7.9.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] tgsi: obey clarified shift behavior
From: Roland Scheidegger srol...@vmware.com
c shifts are undefined for shift counts exceeding (or matching) bit width,
so need to apply a mask (on x86 it actually would usually probably work as
shifts do masking on int domain shifts - unless some auto-vectorizer would
come along at last as simd domain does not mask the shift count).
---
src/gallium/auxiliary/tgsi/tgsi_exec.c | 39 ++--
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c
b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index 3ac6901..d991d4b 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -3198,10 +3198,15 @@ micro_shl(union tgsi_exec_channel *dst,
const union tgsi_exec_channel *src0,
const union tgsi_exec_channel *src1)
{
- dst-u[0] = src0-u[0] src1-u[0];
- dst-u[1] = src0-u[1] src1-u[1];
- dst-u[2] = src0-u[2] src1-u[2];
- dst-u[3] = src0-u[3] src1-u[3];
+ unsigned masked_count;
+ masked_count = src1-u[0] 0x1f;
+ dst-u[0] = src0-u[0] masked_count;
+ masked_count = src1-u[1] 0x1f;
+ dst-u[1] = src0-u[1] masked_count;
+ masked_count = src1-u[2] 0x1f;
+ dst-u[2] = src0-u[2] masked_count;
+ masked_count = src1-u[3] 0x1f;
+ dst-u[3] = src0-u[3] masked_count;
}
static void
@@ -3307,10 +3312,15 @@ micro_ishr(union tgsi_exec_channel *dst,
const union tgsi_exec_channel *src0,
const union tgsi_exec_channel *src1)
{
- dst-i[0] = src0-i[0] src1-i[0];
- dst-i[1] = src0-i[1] src1-i[1];
- dst-i[2] = src0-i[2] src1-i[2];
- dst-i[3] = src0-i[3] src1-i[3];
+ unsigned masked_count;
+ masked_count = src1-i[0] 0x1f;
+ dst-i[0] = src0-i[0] masked_count;
+ masked_count = src1-i[1] 0x1f;
+ dst-i[1] = src0-i[1] masked_count;
+ masked_count = src1-i[2] 0x1f;
+ dst-i[2] = src0-i[2] masked_count;
+ masked_count = src1-i[3] 0x1f;
+ dst-i[3] = src0-i[3] masked_count;
}
static void
@@ -3449,10 +3459,15 @@ micro_ushr(union tgsi_exec_channel *dst,
const union tgsi_exec_channel *src0,
const union tgsi_exec_channel *src1)
{
- dst-u[0] = src0-u[0] src1-u[0];
- dst-u[1] = src0-u[1] src1-u[1];
- dst-u[2] = src0-u[2] src1-u[2];
- dst-u[3] = src0-u[3] src1-u[3];
+ unsigned masked_count;
+ masked_count = src1-u[0] 0x1f;
+ dst-u[0] = src0-u[0] masked_count;
+ masked_count = src1-u[1] 0x1f;
+ dst-u[1] = src0-u[1] masked_count;
+ masked_count = src1-u[2] 0x1f;
+ dst-u[2] = src0-u[2] masked_count;
+ masked_count = src1-u[3] 0x1f;
+ dst-u[3] = src0-u[3] masked_count;
}
static void
--
1.7.9.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] gallium: clarify shift behavior with shift count = 32
Am 30.07.2013 18:13, schrieb srol...@vmware.com: From: Roland Scheidegger srol...@vmware.com Previously, nothing was said what happens with shift counts exceeding bit width of the values to shift. In theory 3 behaviors are possible: 1) undefined (classic c definition) 2) just shift out all bits (so result is zero, or -1 potentially for ashr) 3) mask the shift count to bit width - 1 API's either require 3) or are ok with 1). In particular, GLSL (as well as a couple uninteresting legacy GL extensions) is happy with undefined, whereas both OpenCL and d3d10 require 3). Consequently, most hw also implements 3). So, for simplicity we just specify that 3) is required rather than saying undefined and then needing state trackers to work around it. Also while here specify shift count as a vector, not scalar. As far as I can tell this was a doc bug, neither state trackers nor drivers used scalar shift count. Hmm actually I'm no longer sure this was a simple doc bug. I notice that in fact d3d10 (but not d3d11!) requires a scalar shift count. Though glsl/EXT_gpu_shader4 say it works with both scalar and vector shift counts, and obviously while scalar shift count behavior can be implemented with vector shift count the opposite isn't true. So regardless if this was a doc bug or not, I still propose to change this, and it still seems drivers/state trackers didn't honor this anyway. Roland --- src/gallium/docs/source/tgsi.rst | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/gallium/docs/source/tgsi.rst b/src/gallium/docs/source/tgsi.rst index 0557ce0..8506b7e 100644 --- a/src/gallium/docs/source/tgsi.rst +++ b/src/gallium/docs/source/tgsi.rst @@ -1254,41 +1254,47 @@ Support for these opcodes indicated by PIPE_SHADER_CAP_INTEGERS (all of them?) .. opcode:: SHL - Shift Left + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x src1.x + dst.x = src0.x (0x1f src1.x) - dst.y = src0.y src1.x + dst.y = src0.y (0x1f src1.y) - dst.z = src0.z src1.x + dst.z = src0.z (0x1f src1.z) - dst.w = src0.w src1.x + dst.w = src0.w (0x1f src1.w) .. opcode:: ISHR - Arithmetic Shift Right (of Signed Integer) + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x src1.x + dst.x = src0.x (0x1f src1.x) - dst.y = src0.y src1.x + dst.y = src0.y (0x1f src1.y) - dst.z = src0.z src1.x + dst.z = src0.z (0x1f src1.z) - dst.w = src0.w src1.x + dst.w = src0.w (0x1f src1.w) .. opcode:: USHR - Logical Shift Right + The shift count is masked with 0x1f before the shift is applied. + .. math:: - dst.x = src0.x (unsigned) src1.x + dst.x = src0.x (unsigned) (0x1f src1.x) - dst.y = src0.y (unsigned) src1.x + dst.y = src0.y (unsigned) (0x1f src1.y) - dst.z = src0.z (unsigned) src1.x + dst.z = src0.z (unsigned) (0x1f src1.z) - dst.w = src0.w (unsigned) src1.x + dst.w = src0.w (unsigned) (0x1f src1.w) .. opcode:: UCMP - Integer Conditional Move ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] mesa: Update comments to match newer specs.
Old GL 1.x specs used 'b' but newer specs use 'p'. The line immediately
above the second hunk also uses 'p'.
---
src/mesa/main/mtypes.h | 2 +-
src/mesa/main/texobj.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 008f68b..3d8f359 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1171,7 +1171,7 @@ struct gl_texture_object
GLint MaxLevel; /** max mipmap level, OpenGL 1.2 */
GLint ImmutableLevels; /** ES 3.0 / ARB_texture_view */
GLint _MaxLevel;/** actual max mipmap level (q in the spec) */
- GLfloat _MaxLambda; /** = _MaxLevel - BaseLevel (q - b in spec) */
+ GLfloat _MaxLambda; /** = _MaxLevel - BaseLevel (q - p in spec) */
GLint CropRect[4]; /** GL_OES_draw_texture */
GLenum Swizzle[4]; /** GL_EXT_texture_swizzle */
GLuint _Swizzle; /** same as Swizzle, but SWIZZLE_* format */
diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
index 66377c8..d0fcb12 100644
--- a/src/mesa/main/texobj.c
+++ b/src/mesa/main/texobj.c
@@ -553,7 +553,7 @@ _mesa_test_texobj_completeness( const struct gl_context
*ctx,
t-_MaxLevel = MIN2(t-_MaxLevel, t-MaxLevel);
t-_MaxLevel = MIN2(t-_MaxLevel, maxLevels - 1); /* 'q' in the GL spec */
- /* Compute _MaxLambda = q - b (see the 1.2 spec) used during mipmapping */
+ /* Compute _MaxLambda = q - p in the spec used during mipmapping */
t-_MaxLambda = (GLfloat) (t-_MaxLevel - baseLevel);
if (t-Immutable) {
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/5] glsl: Clean up linker error checking.
The series is Reviewed-by: Ian Romanick ian.d.roman...@intel.com On 07/27/2013 03:59 PM, Paul Berry wrote: I recently discovered a bug in my geometry shader branch wherein we were responding to certain kinds of link errors by calling linker_error(), but the program was still linking successfully. The root cause turned out to be that during the first half of the execution of link_shaders(), it was not sufficient to call linker_error()--the code had to also tell link_shaders() (through boolean return values) that the link had failed. During the second half link_shaders(), calling linker_error() was sufficient. This patch series cleans things up so that calling linker_error() is always sufficient to make the link fail, regardless of where execution is in link_shaders(). The cleanup happens in patch 3/5. In the process of making this fix, I also discovered one case where we were flagging the link error by returning false to link_shaders(), but we weren't calling linker_error(). As a result, the link would fail (as it should) but there would be no error message in the log. That is fixed in patch 2/5. Patch 1/5 removes some dead code which would have otherwise made the clean-up harder to follow. Patch 4/5 improves an error message that was previously vague because we were calling linker_error() from a point where we didn't have much information. Finally, patch 5/5 removes some redundant code. [PATCH 1/5] glsl: Remove bogus check on return value of link_uniform_blocks(). [PATCH 2/5] glsl: Add error message for intrastage interface block mismatch. [PATCH 3/5] glsl: Use a consistent technique for tracking link success/failure. [PATCH 4/5] glsl: Improve error message for interstage interface block mismatch. [PATCH 5/5] glsl: Remove redundant writes to prog-LinkStatus ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 63435] [Regression since 9.0] Flickering in EGL OpenGL full-screen window with swap interval 1
https://bugs.freedesktop.org/show_bug.cgi?id=63435 Ian Romanick i...@freedesktop.org changed: What|Removed |Added Status|NEW |NEEDINFO Assignee|mesa-dev@lists.freedesktop. |kenn...@whitecape.org |org | CC||i...@freedesktop.org --- Comment #10 from Ian Romanick i...@freedesktop.org --- This should be fixed by 0e9549e on master. Could you verify that? -- 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] mesa: Reject VertexAttribPointer() with GL_BGRA and !normalized.
On 07/23/2013 03:50 PM, Matt Turner wrote:
On Mon, Jul 15, 2013 at 6:37 AM, Fredrik Höglund fred...@kde.org wrote:
On Monday 15 July 2013, Kenneth Graunke wrote:
Fixes Piglit's ARB_vertex_attrib_bgra/api-errors test.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/varray.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 529d933..48f15bd 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -463,6 +463,16 @@ _mesa_VertexAttribPointer(GLuint index, GLint size, GLenum
type,
return;
}
+ /* From the GL_ARB_vertex_array_bgra specification:
+* The error INVALID_VALUE is generated by VertexAttribPointer if
+* size is BGRA and normalized is FALSE.
+*/
+ if (size == GL_BGRA !normalized) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ glVertexAttribPointer(BGRA and !normalized));
+ return;
+ }
+
I think this code belongs in update_array(), since it already handles other
BGRA errors, and also checks if the extension is supported.
This also reminds me that we need to decide if we should make the error
code depend on the GL version and entry point.
Both OpenGL 3.3 and GL_ARB_vertex_attrib_binding changes the error code
to GL_INVALID_OPERATION. The extension only does it for the entry points
it adds.
I personally don't think it's worth adding the additional complexity for
that since the conditions under which the errors to be generated haven't
changed.
Ian,
I remember you mentioning when we were showing the interns how to
write piglit tests that the return value seemed wrong. Can you
elaborate here? This is holding up Fredrik's ARB_vertex_attrib_binding
patches.
I believe the ARB_vertex_array_bgra should have specified
INVALID_OPERATION as the error code. Section 2.5 (GL Errors) provides
the general guidance that INVALID_VALUE is generated for Numeric
argument out of range. This is error is that a GLboolean is false
instead of true. Clearly false is in range for GLboolean, so
INVALID_VALUE is not appropriate.
Since OpenGL 3.3 corrects the error code, we should follow that behavior.
Thanks,
Matt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 67516] glTexStorage*() functions don't work properly with proxy textures
https://bugs.freedesktop.org/show_bug.cgi?id=67516 --- Comment #6 from Mikko Juola mik...@gmail.com --- Created attachment 83319 -- https://bugs.freedesktop.org/attachment.cgi?id=83319action=edit patch 1/1 I discovered another proxy texture related bug with multisampling textures. glGetTexLevelParameteriv() would fail for GL_PROXY_TEXTURE_2D_MULTISAMPLE and GL_PROXY_TEXTURE_2D_MULTISAMPLE_ARRAY because one of the error checking functions seems to be blissfully unaware these targets exist. I attached another patch to fix this one. This is technically a different bug to the texture storage proxy texture bugs but since we are on the topic of proxy textures, I attached this one here. -- 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 09/34] glsl: add ir_emitvertex and ir_endprim instruction types
On 07/28/2013 11:03 PM, Paul Berry wrote:
From: Bryan Cain bryanca...@gmail.com
These correspond to the EmitVertex and EndPrimitive functions in GLSL.
v2 (Paul Berry stereotype...@gmail.com): Add stub implementations of
new pure visitor functions to i965's vec4_visitor and fs_visitor
classes.
I would prefer to see these called:
ir_emit_vertex
ir_end_primitive
The GLSL constructs are called EmitVertex and EndPrimitive, and so the
obvious conversion from CamelCase to lowercase_with_underscores would
result in ir_emit_vertex/ir_end_primitive.
I hate to make that suggestion, since it means a lot of pointless churn,
but...somehow ir_emitvertex just bothers me every time I see it.
diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
index ca973a5..44c4933 100644
--- a/src/glsl/ir_print_visitor.cpp
+++ b/src/glsl/ir_print_visitor.cpp
@@ -539,3 +539,15 @@ ir_print_visitor::visit(ir_loop_jump *ir)
{
printf(%s, ir-is_break() ? break : continue);
}
+
+void
+ir_print_visitor::visit(ir_emitvertex *ir)
+{
+ printf((emitvertex));
+}
+
+void
+ir_print_visitor::visit(ir_endprim *ir)
+{
+ printf((endprim));
+}
Could we call these (emit-vertex) and either (end-prim) or
(end-primitive)? Those are the Scheme-like names.
[snip]
diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
index 51534ca..f8fb52d 100644
--- a/src/glsl/ir_reader.cpp
+++ b/src/glsl/ir_reader.cpp
[snip]
@@ -355,6 +357,10 @@ ir_reader::read_instruction(s_expression *expr, ir_loop
*loop_ctx)
inst = read_return(list);
} else if (strcmp(tag-value(), function) == 0) {
inst = read_function(list, false);
+ } else if (strcmp(tag-value(), emitvertex) == 0) {
+ inst = read_emitvertex(list);
+ } else if (strcmp(tag-value(), endprim) == 0) {
+ inst = read_endprim(list);
} else {
inst = read_rvalue(list);
if (inst == NULL)
These would need to be updated for the new names.
@@ -1065,3 +1071,27 @@ ir_reader::read_texture(s_expression *expr)
};
return tex;
}
+
+ir_emitvertex *
+ir_reader::read_emitvertex(s_expression *expr)
+{
+ s_pattern pat[] = { emitvertex };
Ditto.
+
+ if (MATCH(expr, pat)) {
+ return new(mem_ctx) ir_emitvertex();
+ }
+ ir_read_error(NULL, when reading emitvertex);
+ return NULL;
+}
+
+ir_endprim *
+ir_reader::read_endprim(s_expression *expr)
+{
+ s_pattern pat[] = { endprim };
Ditto.
+
+ if (MATCH(expr, pat)) {
+ return new(mem_ctx) ir_endprim();
+ }
+ ir_read_error(NULL, when reading emitvertex);
+ return NULL;
+}
diff --git a/src/glsl/ir_visitor.h b/src/glsl/ir_visitor.h
index bd47ef7..af0f986 100644
--- a/src/glsl/ir_visitor.h
+++ b/src/glsl/ir_visitor.h
@@ -63,6 +63,8 @@ public:
virtual void visit(class ir_if *) = 0;
virtual void visit(class ir_loop *) = 0;
virtual void visit(class ir_loop_jump *) = 0;
+ virtual void visit(class ir_emitvertex *) = 0;
+ virtual void visit(class ir_endprim *) = 0;
/*@}*/
};
@@ -81,6 +83,8 @@ public:
virtual void visit(class ir_assignment *) {}
virtual void visit(class ir_constant *) {}
virtual void visit(class ir_call *) {}
+ virtual void visit(class ir_emitvertex *) {}
+ virtual void visit(class ir_endprim *) {}
};
#endif /* __cplusplus */
diff --git a/src/glsl/lower_output_reads.cpp b/src/glsl/lower_output_reads.cpp
index b93e254..bc633ba 100644
--- a/src/glsl/lower_output_reads.cpp
+++ b/src/glsl/lower_output_reads.cpp
@@ -50,6 +50,7 @@ public:
output_read_remover();
~output_read_remover();
virtual ir_visitor_status visit(class ir_dereference_variable *);
+ virtual ir_visitor_status visit(class ir_emitvertex *);
virtual ir_visitor_status visit_leave(class ir_return *);
virtual ir_visitor_status visit_leave(class ir_function_signature *);
};
@@ -117,7 +118,9 @@ copy(void *ctx, ir_variable *output, ir_variable *temp)
return new(ctx) ir_assignment(lhs, rhs);
}
-/** Insert a copy-back assignment before a return statement */
+/** Insert a copy-back assignment before a return statement or a call to
+ * EmitVertex().
+ */
static void
emit_return_copy(const void *key, void *data, void *closure)
{
@@ -141,6 +144,14 @@ output_read_remover::visit_leave(ir_return *ir)
}
ir_visitor_status
+output_read_remover::visit(ir_emitvertex *ir)
+{
+ hash_table_call_foreach(replacements, emit_return_copy, ir);
+ hash_table_clear(replacements);
+ return visit_continue;
+}
+
+ir_visitor_status
output_read_remover::visit_leave(ir_function_signature *sig)
{
if (strcmp(sig-function_name(), main) != 0)
diff --git a/src/glsl/opt_dead_code_local.cpp b/src/glsl/opt_dead_code_local.cpp
index 8c31802..72a4b38 100644
--- a/src/glsl/opt_dead_code_local.cpp
+++ b/src/glsl/opt_dead_code_local.cpp
@@ -114,6 +114,23 @@ public:
return visit_continue_with_parent;
}
+ virtual ir_visitor_status visit(ir_emitvertex *ir)
+ {
+ /* For the purpose of
Re: [Mesa-dev] [PATCH 10/34] glsl: add builtins for geometry shaders.
On 07/28/2013 11:03 PM, Paul Berry wrote:
From: Bryan Cain bryanca...@gmail.com
v2 (Paul Berry stereotype...@gmail.com): Account for rework of
builtin_variables.cpp. Use INTERP_QUALIFIER_FLAT for gl_PrimitiveID
so that it will obey provoking vertex conventions. Convert to GLSL
1.50 style geometry shaders.
---
src/glsl/builtin_variables.cpp | 11 +--
src/glsl/builtins/ir/EmitVertex.ir | 5 +
src/glsl/builtins/ir/EndPrimitive.ir | 5 +
src/glsl/builtins/profiles/150.geom | 3 +++
src/glsl/builtins/tools/generate_builtins.py | 6 --
5 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 src/glsl/builtins/ir/EmitVertex.ir
create mode 100644 src/glsl/builtins/ir/EndPrimitive.ir
create mode 100644 src/glsl/builtins/profiles/150.geom
diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
index 1e88b6a..9d927a4 100644
--- a/src/glsl/builtin_variables.cpp
+++ b/src/glsl/builtin_variables.cpp
@@ -686,8 +686,10 @@ builtin_variable_generator::generate_gs_special_vars()
* the specific case of gl_PrimitiveIDIn. So we don't need to treat
* gl_PrimitiveIDIn as an {ARB,EXT}_geometry_shader4-only variable.
*/
- add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveIDIn);
- add_output(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveID);
+ add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveIDIn)
+ -interpolation = INTERP_QUALIFIER_FLAT;
+ add_output(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveID)
+ -interpolation = INTERP_QUALIFIER_FLAT;
}
This took a moment to understand :)
Could we instead use a temporary? Something like:
ir_variable *var;
var = add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveIDIn);
var-interpolation = INTERP_QUALIFIER_FLAT;
var = add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveID);
var-interpolation = INTERP_QUALIFIER_FLAT;
etc.
@@ -702,6 +704,11 @@ builtin_variable_generator::generate_fs_special_vars()
if (state-is_version(120, 100))
add_input(VARYING_SLOT_PNTC, vec2_t, gl_PointCoord);
+ if (state-is_version(150, 0)) {
+ add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, gl_PrimitiveID)
+ -interpolation = INTERP_QUALIFIER_FLAT;
Ditto.
+ }
+
/* gl_FragColor and gl_FragData were deprecated starting in desktop GLSL
* 1.30, and were relegated to the compatibility profile in GLSL 4.20.
* They were removed from GLSL ES 3.00.
diff --git a/src/glsl/builtins/ir/EmitVertex.ir
b/src/glsl/builtins/ir/EmitVertex.ir
new file mode 100644
index 000..e08ec62
--- /dev/null
+++ b/src/glsl/builtins/ir/EmitVertex.ir
@@ -0,0 +1,5 @@
+((function EmitVertex
+ (signature void
+(parameters)
+((emitvertex)))
+))
diff --git a/src/glsl/builtins/ir/EndPrimitive.ir
b/src/glsl/builtins/ir/EndPrimitive.ir
new file mode 100644
index 000..ea6ecfd
--- /dev/null
+++ b/src/glsl/builtins/ir/EndPrimitive.ir
@@ -0,0 +1,5 @@
+((function EndPrimitive
+ (signature void
+(parameters)
+((endprim)))
+))
diff --git a/src/glsl/builtins/profiles/150.geom
b/src/glsl/builtins/profiles/150.geom
new file mode 100644
index 000..d2ab891
--- /dev/null
+++ b/src/glsl/builtins/profiles/150.geom
@@ -0,0 +1,3 @@
+#version 150
+void EmitVertex();
+void EndPrimitive();
diff --git a/src/glsl/builtins/tools/generate_builtins.py
b/src/glsl/builtins/tools/generate_builtins.py
index 85bd5dd..54c5a49 100755
--- a/src/glsl/builtins/tools/generate_builtins.py
+++ b/src/glsl/builtins/tools/generate_builtins.py
@@ -125,7 +125,7 @@ def write_profiles():
def get_profile_list():
profile_files = []
-for extension in ['glsl', 'frag', 'vert']:
+for extension in ['glsl', 'frag', 'vert', 'geom']:
path_glob = path.join(
path.join(builtins_dir, 'profiles'), '*.' + extension)
profile_files.extend(glob(path_glob))
@@ -279,10 +279,12 @@ _mesa_glsl_initialize_functions(struct
_mesa_glsl_parse_state *state)
check = 'state-target == vertex_shader '
elif profile.endswith('_frag'):
check = 'state-target == fragment_shader '
+elif profile.endswith('_geom'):
+check = 'state-target == geometry_shader '
else:
check = ''
-version = re.sub(r'_(glsl|vert|frag)$', '', profile)
+version = re.sub(r'_(glsl|vert|frag|geom)$', '', profile)
if version[0].isdigit():
is_es = version.endswith('es')
if is_es:
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: handle 2D texture arrays in get_tex_rgba_compressed()
Brian Paul bri...@vmware.com writes: If we call glGetTexImage() for a compressed 2D texture array we need to loop over all the slices. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66850 NOTE: This is a candidate for the 9.x branches. Cc: mesa-sta...@lists.freedesktop.org Thanks. I've now picked all three commits from this series to a 9.1 branch which I plan to push out soon. -Carl -- carl.d.wo...@intel.com pgpuwKLlbZbNx.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] glsl: Handle empty if statement encountered during loop analysis.
Paul Berry stereotype...@gmail.com writes: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64330 CC: mesa-sta...@lists.freedesktop.org Thanks. I've now picked this over to a 9.1 branch which I plan to push out soon. -Carl -- carl.d.wo...@intel.com pgpr06gKPpBHA.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] i965/vs: Fix flaky texture swizzling
Chris Forbes chr...@ijw.co.nz writes: NOTE: This is a candidate for stable branches. Thanks. I've now picked this to a 9.1 branch which I plan to push out soon. -Carl -- carl.d.wo...@intel.com pgp6AduBOvuys.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: Classify layout like other identifiers.
Kenneth Graunke kenn...@whitecape.org writes: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64087 .. Cc: mesa-sta...@lists.freedesktop.org Picked for 9.1. Thanks. -Carl -- carl.d.wo...@intel.com pgpn3imSF9FUf.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Should we add new error cases to stable releases?
Hi Ken,
In the last stable-release cycle you argued for not cherry picking
commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273 on the grounds that
we shouldn't change mesa to be strictly more strict in a stable release.
That is, adding a new error case to the compiler cannot actually cause a
valid program to start working, (which would be a nice bug fix), but
could potentially cause an invalid program to stop working (which could
be regarded as a regression).
The same reasoning seems to disqualify the below patch. Do you agree?
Anyone else have input on this issue? If we agree on the rationale, I'd
like to codify it with some language in our documentation for
stable-branch criteria. Something like:
Note: Not all bug fixes are automatically suitable for the
stable branch.
For example, a patch that makes the implementation more strict,
(such as detecting some invalid GLSL code that was previously
silently accepted), can be rejected by the stable-branch
maintainer. The rationale is that a patch like this can cause a
program which was previously working as desired to now stop
working due to the new error case. From the point of view of the
user of such an application, that's a regression that we do not
want to allow during a stable release.
-Carl
commit 17856726c94000bf16156f7f9acea77a271a6005
Author: Kenneth Graunke kenn...@whitecape.org
Date: Fri Jul 26 21:18:56 2013 -0700
glsl: Disallow auxiliary storage qualifiers on FS outputs.
This has always been an error; we just forgot to check for it.
Fixes Piglit's no-aux-qual-on-fs-output.frag.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67333
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
Reviewed-by: Matt Turner matts...@gmail.com
Cc: mesa-sta...@lists.freedesktop.org
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 2569cde..598da92 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2927,6 +2927,20 @@ ast_declarator_list::hir(exec_list *instructions,
'centroid in' cannot be used in a vertex shader);
}
+ /* Section 4.3.6 of the GLSL 1.30 specification states:
+ * It is an error to use centroid out in a fragment shader.
+ *
+ * The GL_ARB_shading_language_420pack extension specification states:
+ * It is an error to use auxiliary storage qualifiers or interpolation
+ * qualifiers on an output in a fragment shader.
+ */
+ if (state-target == fragment_shader
+ this-type-qualifier.flags.q.out
+ this-type-qualifier.has_auxiliary_storage()) {
+ _mesa_glsl_error(loc, state,
+ auxiliary storage qualifiers cannot be used on
+ fragment shader outputs);
+ }
/* Precision qualifiers exists only in GLSL versions 1.00 and = 1.30.
*/
pgpR63sEqyVGx.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Request for update of commit proposed for stable branch
Thank you. Sorry for not replying earlier. Marek On Tue, Jul 30, 2013 at 12:54 AM, Carl Worth cwo...@cworth.org wrote: Carl Worth cwo...@cworth.org writes: I looked closer at the patch and it seems that it's fairly easy to backport to the 9.1 branch as seen below. And looking at the mesa-stable@ list, I see that you already sent an identical backport. So I'll definitely apply that for now. -Carl ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] tgsi: add ucmp to the list of opcodes
we forgot to add ucmp to the list of opcodes, so it was never generated for ureg. Signed-off-by: Zack Rusin za...@vmware.com --- src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h |2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h index b87c4b1..93ec0b5 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h +++ b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h @@ -198,6 +198,8 @@ OP12(SVIEWINFO) OP13(SAMPLE_POS) OP12(SAMPLE_INFO) +OP13(UCMP) + #undef OP00 #undef OP01 -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] tgsi: add ucmp to the list of opcodes
Am 30.07.2013 22:12, schrieb Zack Rusin: we forgot to add ucmp to the list of opcodes, so it was never generated for ureg. Signed-off-by: Zack Rusin za...@vmware.com --- src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h |2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h index b87c4b1..93ec0b5 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h +++ b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h @@ -198,6 +198,8 @@ OP12(SVIEWINFO) OP13(SAMPLE_POS) OP12(SAMPLE_INFO) +OP13(UCMP) + #undef OP00 #undef OP01 Looks good (though ucmp still has somewhat undefined behavior wrt src modifiers but doesn't really matter here). Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallivm: obey clarified shift behavior
From: Roland Scheidegger srol...@vmware.com
llvm shifts are undefined for shift counts exceeding (or matching) bit width,
so need to apply a mask for the tgsi shift instructions.
v2: only use mask for the tgsi shift instructions, not for the build shift
helpers. None of the internal callers need this behavior, and while llvm can
optimize away the masking for constants there are legitimate cases where it
might not be able to do so even if we know that shift count must be smaller
than type width (currently all such callers do not use the build shift
helpers).
---
src/gallium/auxiliary/gallivm/lp_bld_bitarit.c |8 +--
src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 24 +++-
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
b/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
index 97ae162..9892d7a 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_bitarit.c
@@ -168,6 +168,7 @@ lp_build_not(struct lp_build_context *bld, LLVMValueRef a)
/**
* Shift left.
+ * Result is undefined if the shift count is not smaller than the type width.
*/
LLVMValueRef
lp_build_shl(struct lp_build_context *bld, LLVMValueRef a, LLVMValueRef b)
@@ -189,6 +190,7 @@ lp_build_shl(struct lp_build_context *bld, LLVMValueRef a,
LLVMValueRef b)
/**
* Shift right.
+ * Result is undefined if the shift count is not smaller than the type width.
*/
LLVMValueRef
lp_build_shr(struct lp_build_context *bld, LLVMValueRef a, LLVMValueRef b)
@@ -214,23 +216,25 @@ lp_build_shr(struct lp_build_context *bld, LLVMValueRef
a, LLVMValueRef b)
/**
* Shift left with immediate.
+ * The immediate shift count must be smaller than the type width.
*/
LLVMValueRef
lp_build_shl_imm(struct lp_build_context *bld, LLVMValueRef a, unsigned imm)
{
LLVMValueRef b = lp_build_const_int_vec(bld-gallivm, bld-type, imm);
- assert(imm = bld-type.width);
+ assert(imm bld-type.width);
return lp_build_shl(bld, a, b);
}
/**
* Shift right with immediate.
+ * The immediate shift count must be smaller than the type width.
*/
LLVMValueRef
lp_build_shr_imm(struct lp_build_context *bld, LLVMValueRef a, unsigned imm)
{
LLVMValueRef b = lp_build_const_int_vec(bld-gallivm, bld-type, imm);
- assert(imm = bld-type.width);
+ assert(imm bld-type.width);
return lp_build_shr(bld, a, b);
}
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
index d16ccae..f461661 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
@@ -1203,8 +1203,12 @@ ishr_emit_cpu(
struct lp_build_tgsi_context * bld_base,
struct lp_build_emit_data * emit_data)
{
- emit_data-output[emit_data-chan] = lp_build_shr(bld_base-int_bld,
- emit_data-args[0], emit_data-args[1]);
+ struct lp_build_context *int_bld = bld_base-int_bld;
+ LLVMValueRef mask = lp_build_const_vec(int_bld-gallivm, int_bld-type,
+ int_bld-type.width - 1);
+ LLVMValueRef masked_count = lp_build_and(int_bld, emit_data-args[1], mask);
+ emit_data-output[emit_data-chan] = lp_build_shr(int_bld,
emit_data-args[0],
+ masked_count);
}
/* TGSI_OPCODE_ISLT (CPU Only) */
@@ -1439,8 +1443,12 @@ shl_emit_cpu(
struct lp_build_tgsi_context * bld_base,
struct lp_build_emit_data * emit_data)
{
- emit_data-output[emit_data-chan] = lp_build_shl(bld_base-uint_bld,
- emit_data-args[0], emit_data-args[1]);
+ struct lp_build_context *uint_bld = bld_base-uint_bld;
+ LLVMValueRef mask = lp_build_const_vec(uint_bld-gallivm, uint_bld-type,
+ uint_bld-type.width - 1);
+ LLVMValueRef masked_count = lp_build_and(uint_bld, emit_data-args[1],
mask);
+ emit_data-output[emit_data-chan] = lp_build_shl(uint_bld,
emit_data-args[0],
+ masked_count);
}
/* TGSI_OPCODE_SIN (CPU Only) */
@@ -1647,8 +1655,12 @@ ushr_emit_cpu(
struct lp_build_tgsi_context * bld_base,
struct lp_build_emit_data * emit_data)
{
- emit_data-output[emit_data-chan] = lp_build_shr(bld_base-uint_bld,
- emit_data-args[0], emit_data-args[1]);
+ struct lp_build_context *uint_bld = bld_base-uint_bld;
+ LLVMValueRef mask = lp_build_const_vec(uint_bld-gallivm, uint_bld-type,
+ uint_bld-type.width - 1);
+ LLVMValueRef masked_count = lp_build_and(uint_bld, emit_data-args[1],
mask);
+ emit_data-output[emit_data-chan] = lp_build_shr(uint_bld,
emit_data-args[0],
+ masked_count);
}
/* TGSI_OPCODE_ISLT (CPU Only) */
--
1.7.9.5
Re: [Mesa-dev] Should we add new error cases to stable releases?
Matt Turner matts...@gmail.com writes: I think that this patch /should/ go in, since it was a fix for a new extension. 9.2 will be the first version of Mesa to ship with support for ARB_shading_language_420pack, so the branch should contain bug fixes for the extension. Thanks. And yes, that reasoning makes sense. Meanwhile, I was actually working only on the 9.1 branch. So you've also answered my question that we don't need the patch there. -Carl -- carl.d.wo...@intel.com pgpSGdZFiSoqI.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 V3 2/5] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3
On 14 July 2013 02:39, Chris Forbes chr...@ijw.co.nz wrote:
Previously the SF only handled the builtin color varying specially.
This patch generalizes that support to cover user-defined varyings,
driven by the interpolation mode array set up alongside the VUE map.
Based on the following patches from Olivier Galibert:
- http://lists.freedesktop.org/archives/mesa-dev/2012-July/024335.html
- http://lists.freedesktop.org/archives/mesa-dev/2012-July/024339.html
With this patch, all the GLSL 1.3 interpolation tests that do not clip
(spec/glsl-1.30/execution/interpolation/*-none.shader_test) pass.
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/brw_fs.cpp| 2 +-
src/mesa/drivers/dri/i965/brw_sf.c | 9 +-
src/mesa/drivers/dri/i965/brw_sf.h | 2 +-
src/mesa/drivers/dri/i965/brw_sf_emit.c | 187
4 files changed, 106 insertions(+), 94 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index afd29de..a81e97f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -1048,7 +1048,7 @@ fs_visitor::emit_general_interpolation(ir_variable
*ir)
inst-predicate = BRW_PREDICATE_NORMAL;
inst-predicate_inverse = true;
}
- if (brw-gen 6) {
+ if (brw-gen 6 interpolation_mode ==
INTERP_QUALIFIER_SMOOTH) {
emit(BRW_OPCODE_MUL, attr, attr, this-pixel_w);
}
attr.reg_offset++;
diff --git a/src/mesa/drivers/dri/i965/brw_sf.c
b/src/mesa/drivers/dri/i965/brw_sf.c
index 7cd383c..f7725ed 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.c
+++ b/src/mesa/drivers/dri/i965/brw_sf.c
@@ -138,6 +138,7 @@ brw_upload_sf_prog(struct brw_context *brw)
struct brw_sf_prog_key key;
/* _NEW_BUFFERS */
bool render_to_fbo = _mesa_is_user_fbo(ctx-DrawBuffer);
+ int i;
memset(key, 0, sizeof(key));
@@ -190,7 +191,13 @@ brw_upload_sf_prog(struct brw_context *brw)
key.sprite_origin_lower_left = true;
/* _NEW_LIGHT | _NEW_PROGRAM */
- key.do_flat_shading = (ctx-Light.ShadeModel == GL_FLAT);
+ key.has_flat_shading = 0;
+ for (i = 0; i BRW_VARYING_SLOT_COUNT; i++) {
+ if (brw-interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
+ key.has_flat_shading = 1;
+ break;
+ }
+ }
Having one part of the key that is just derived from another part of the
key is inefficient, since it means that this computation has to happen
every time the state atom ir executed.
I'd recommend moving has_flat_shading into brw_sf_compile, that way we can
compute it in compile_sf_prog(), which only has to execute when the key
changes.
key.do_twoside_color = ((ctx-Light.Enabled
ctx-Light.Model.TwoSide) ||
ctx-VertexProgram._TwoSideEnabled);
diff --git a/src/mesa/drivers/dri/i965/brw_sf.h
b/src/mesa/drivers/dri/i965/brw_sf.h
index 55a860d..9bfa994 100644
--- a/src/mesa/drivers/dri/i965/brw_sf.h
+++ b/src/mesa/drivers/dri/i965/brw_sf.h
@@ -50,7 +50,7 @@ struct brw_sf_prog_key {
uint8_t point_sprite_coord_replace;
GLuint primitive:2;
GLuint do_twoside_color:1;
- GLuint do_flat_shading:1;
+ GLuint has_flat_shading:1;
GLuint frontface_ccw:1;
GLuint do_point_sprite:1;
GLuint do_point_coord:1;
diff --git a/src/mesa/drivers/dri/i965/brw_sf_emit.c
b/src/mesa/drivers/dri/i965/brw_sf_emit.c
index bd68f68..63e077d 100644
--- a/src/mesa/drivers/dri/i965/brw_sf_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_sf_emit.c
@@ -44,6 +44,15 @@
/**
+ * Determine the vue slot corresponding to the given half of the given
register.
+ */
+static inline int vert_reg_to_vue_slot(struct brw_sf_compile *c, GLuint
reg,
+ int half)
+{
+ return (reg + c-urb_entry_read_offset) * 2 + half;
+}
+
+/**
* Determine the varying corresponding to the given half of the given
* register. half=0 means the first half of a register, half=1 means the
* second half.
@@ -51,11 +60,24 @@
static inline int vert_reg_to_varying(struct brw_sf_compile *c, GLuint
reg,
int half)
{
- int vue_slot = (reg + c-urb_entry_read_offset) * 2 + half;
+ int vue_slot = vert_reg_to_vue_slot(c, reg, half);
return c-vue_map.slot_to_varying[vue_slot];
}
/**
+ * Determine the register corresponding to the given vue slot
+ */
+static struct brw_reg get_vue_slot(struct brw_sf_compile *c,
+ struct brw_reg vert,
+ int vue_slot)
+{
+ GLuint off = vue_slot / 2 - c-urb_entry_read_offset;
+ GLuint sub = vue_slot % 2;
+
+ return brw_vec4_grf(vert.nr + off, sub * 4);
+}
+
+/**
* Determine the register corresponding to the given varying.
*/
static struct brw_reg
Re: [Mesa-dev] [Mesa-stable] Should we add new error cases to stable releases?
On 07/30/2013 12:13 PM, Carl Worth wrote:
Hi Ken,
In the last stable-release cycle you argued for not cherry picking
commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273 on the grounds that
we shouldn't change mesa to be strictly more strict in a stable release.
That is, adding a new error case to the compiler cannot actually cause a
valid program to start working, (which would be a nice bug fix), but
could potentially cause an invalid program to stop working (which could
be regarded as a regression).
The same reasoning seems to disqualify the below patch. Do you agree?
Maybe. If both NVIDIA and AMD correctly generate the error, the
probability of any application using it is epsilon. We should collect
that information and add it to the commit message.
I also think this patch may have just been a candidate for 9.2. :/
Anyone else have input on this issue? If we agree on the rationale, I'd
like to codify it with some language in our documentation for
stable-branch criteria. Something like:
Note: Not all bug fixes are automatically suitable for the
stable branch.
For example, a patch that makes the implementation more strict,
(such as detecting some invalid GLSL code that was previously
silently accepted), can be rejected by the stable-branch
maintainer. The rationale is that a patch like this can cause a
program which was previously working as desired to now stop
working due to the new error case. From the point of view of the
user of such an application, that's a regression that we do not
want to allow during a stable release.
-Carl
commit 17856726c94000bf16156f7f9acea77a271a6005
Author: Kenneth Graunke kenn...@whitecape.org
Date: Fri Jul 26 21:18:56 2013 -0700
glsl: Disallow auxiliary storage qualifiers on FS outputs.
This has always been an error; we just forgot to check for it.
Fixes Piglit's no-aux-qual-on-fs-output.frag.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67333
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
Reviewed-by: Matt Turner matts...@gmail.com
Cc: mesa-sta...@lists.freedesktop.org
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 2569cde..598da92 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2927,6 +2927,20 @@ ast_declarator_list::hir(exec_list *instructions,
'centroid in' cannot be used in a vertex shader);
}
+ /* Section 4.3.6 of the GLSL 1.30 specification states:
+ * It is an error to use centroid out in a fragment shader.
+ *
+ * The GL_ARB_shading_language_420pack extension specification states:
+ * It is an error to use auxiliary storage qualifiers or interpolation
+ * qualifiers on an output in a fragment shader.
+ */
+ if (state-target == fragment_shader
+ this-type-qualifier.flags.q.out
+ this-type-qualifier.has_auxiliary_storage()) {
+ _mesa_glsl_error(loc, state,
+ auxiliary storage qualifiers cannot be used on
+ fragment shader outputs);
+ }
/* Precision qualifiers exists only in GLSL versions 1.00 and = 1.30.
*/
___
mesa-stable mailing list
mesa-sta...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3 2/5] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3
On Wed, Jul 31, 2013 at 8:53 AM, Paul Berry stereotype...@gmail.com wrote: Can you point me to some VS code that does this? I thought that if the VS wrote only to gl_BackColor, then the VUE map would only contain a slot for gl_BackColor. The VS now does this: diff --git a/src/mesa/drivers/dri/i965/ brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index 60b40c5..5b8173d 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -277,6 +277,12 @@ do_vs_prog(struct brw_context *brw, if (c.key.point_coord_replace (1 i)) outputs_written |= BITFIELD64_BIT(VARYING_SLOT_TEX0 + i); } + + /* if back colors are written, allocate slots for front colors too */ + if (outputs_written BITFIELD64_BIT(VARYING_SLOT_BFC0)) + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL0); + if (outputs_written BITFIELD64_BIT(VARYING_SLOT_BFC1)) + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL1); } brw_compute_vue_map(brw, prog_data.base.vue_map, outputs_written, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/15] mesa, glsl, st/dri: add a new driconf option force_glsl_version for Unigine
Hi Kenneth, Sorry I haven't had time to test Heaven 4.0 and I'm currently using the radeonsi driver, which only supports GL 2.1, GLSL 1.30, and it isn't in good shape to run Heaven at the moment. Besides that, Heaven 4.0 throws an error (X Error of failed request: GLXBadFBConfig), while Heaven 3.0 at least starts and crashes in the driver due to lack of register allocation. Anyway, I think we should support Heaven 3.0 despite its bugs, and that's what this patch is all about. Marek On Fri, Jul 19, 2013 at 7:57 PM, Kenneth Graunke kenn...@whitecape.org wrote: On 07/19/2013 08:18 AM, Marek Olšák wrote: [snip] diff --git a/src/mesa/drivers/dri/common/drirc b/src/mesa/drivers/dri/common/drirc index 7c2d3ba..b5430ed 100644 --- a/src/mesa/drivers/dri/common/drirc +++ b/src/mesa/drivers/dri/common/drirc @@ -1,3 +1,27 @@ +!-- + + +Application bugs worked around in this file: + + +* Various Unigine products don't use the #version and #extension GLSL + directives, meaning they only get GLSL 1.10 and no extensions for their + shaders. + Enabling all extensions for Unigine fixes most issues, but the GLSL version + is still 1.10. + +* Unigine Heaven 3.0 with ARB_texture_multisample uses a ivec4 * vec4 + expression, which fails to compile with GLSL 1.10. + Adding #version 130 fixes this. + +* Unigine Heaven 3.0 with ARB_shader_bit_encoding uses the uint keyword, which + fails to compile with GLSL 1.10. + Adding #version 130 fixes this. Have you tried Unigine Heaven 4.0? You need to: export MESA_GL_VERSION_OVERRIDE=3.2 export MESA_GLSL_VERSION_OVERRIDE=150 but then it works fine, at least on i965, without any workarounds at all. It actually seems to render correctly and everything. Heaven 3.0 never did (maybe I need the -ARB_shader_bit_encoding workaround too). Valley and OilRush still have one bug with blend_func_extended. --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] Request for more information for stable-branch patches
Ian Romanick i...@freedesktop.org writes: Foo. This is an issue that Tom pointed out before we started doing the mesa-stable list, and it seems that it does need solving. Right. I had disregarded the concern because I hadn't realized that there was a large stabilization phase for a major release that happens on a branch rather than on master. So we definitely do need to fix this. I can think of a couple somewhat ugly ways to handle this when patches are sent just to the mesa-stable list, but I can't think of anything that would be acceptable for patches sent simultaneously to mesa-dev. :( How about this: CC: mesa-sta...@lists.freedesktop.org CC: 9.1 mesa-sta...@lists.freedesktop.org CC: 9.2 mesa-sta...@lists.freedesktop.org ? I just tested and git-send-email does just fine with that, (it just copies everything to the CC field of the email message). And that should give us the information we need to know what to do. -Carl -- carl.d.wo...@intel.com pgp7GqRom_IgJ.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 V3 2/5] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3
It seems a shame to lose this optimization. Can we move the call to count_flatshaded_attributes() up to the declaration of nr, and then we can keep this? This function isn't even called if there is no flat-shading to do. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3 2/5] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3
On 30 July 2013 14:06, Chris Forbes chr...@ijw.co.nz wrote: On Wed, Jul 31, 2013 at 8:53 AM, Paul Berry stereotype...@gmail.com wrote: Can you point me to some VS code that does this? I thought that if the VS wrote only to gl_BackColor, then the VUE map would only contain a slot for gl_BackColor. The VS now does this: diff --git a/src/mesa/drivers/dri/i965/ brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index 60b40c5..5b8173d 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -277,6 +277,12 @@ do_vs_prog(struct brw_context *brw, if (c.key.point_coord_replace (1 i)) outputs_written |= BITFIELD64_BIT(VARYING_SLOT_TEX0 + i); } + + /* if back colors are written, allocate slots for front colors too */ + if (outputs_written BITFIELD64_BIT(VARYING_SLOT_BFC0)) + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL0); + if (outputs_written BITFIELD64_BIT(VARYING_SLOT_BFC1)) + outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL1); } brw_compute_vue_map(brw, prog_data.base.vue_map, outputs_written, Aha! That's what I was missing, thanks. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3 2/5] i965 Gen4/5: Generalize SF interpolation setup for GLSL1.3
On 30 July 2013 14:24, Chris Forbes chr...@ijw.co.nz wrote: It seems a shame to lose this optimization. Can we move the call to count_flatshaded_attributes() up to the declaration of nr, and then we can keep this? This function isn't even called if there is no flat-shading to do. Ok, I see. Sorry for the noise. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3 3/5] i965 Gen4/5: clip: correctly handle flat varyings
On 14 July 2013 02:39, Chris Forbes chr...@ijw.co.nz wrote:
Previously we only gave special treatment to the builtin color varyings.
This patch adds support for arbitrary flat-shaded varyings, which is
required for GLSL 1.30.
Based on Olivier Galibert's patch from last year:
http://lists.freedesktop.org/archives/mesa-dev/2012-July/024340.html
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/brw_clip.c | 13 +--
src/mesa/drivers/dri/i965/brw_clip.h | 6 ++--
src/mesa/drivers/dri/i965/brw_clip_line.c | 6 ++--
src/mesa/drivers/dri/i965/brw_clip_tri.c | 18 +-
src/mesa/drivers/dri/i965/brw_clip_unfilled.c | 2 +-
src/mesa/drivers/dri/i965/brw_clip_util.c | 51
+++
6 files changed, 40 insertions(+), 56 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
b/src/mesa/drivers/dri/i965/brw_clip.c
index 488e0a0..7df4b18 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -186,6 +186,7 @@ brw_upload_clip_prog(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
struct brw_clip_prog_key key;
+ int i;
memset(key, 0, sizeof(key));
@@ -200,8 +201,16 @@ brw_upload_clip_prog(struct brw_context *brw)
key.primitive = brw-reduced_primitive;
/* BRW_NEW_VUE_MAP_GEOM_OUT */
key.attrs = brw-vue_map_geom_out.slots_valid;
- /* _NEW_LIGHT */
- key.do_flat_shading = (ctx-Light.ShadeModel == GL_FLAT);
+
+ /* BRW_NEW_FRAGMENT_PROGRAM, _NEW_LIGHT */
+ key.has_flat_shading = 0;
+ for (i = 0; i brw-vue_map_geom_out.num_slots; i++) {
+ if (key.interpolation_mode[i] == INTERP_QUALIFIER_FLAT) {
+ key.has_flat_shading = 1;
+ break;
+ }
+ }
Similar to my comment on patch 2/5, I'd prefer to see this for() loop moved
into compile_clip_prog() (and key.has_flat_shading moved into
brw_clip_compile) so that we only do this computation when compiling the
clip program.
In fact, since this is the same computation that patch 2/5 added to SF, we
should probably move it to its own function to avoid code duplication.
With that fixed, this patch is:
Reviewed-by: Paul Berry stereotype...@gmail.com
+
key.pv_first = (ctx-Light.ProvokingVertex ==
GL_FIRST_VERTEX_CONVENTION);
/* _NEW_TRANSFORM (also part of VUE map)*/
key.nr_userclip = _mesa_bitcount_64(ctx-Transform.ClipPlanesEnabled);
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h
b/src/mesa/drivers/dri/i965/brw_clip.h
index fcbe2a0..656254b 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -46,7 +46,7 @@ struct brw_clip_prog_key {
unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
GLuint primitive:4;
GLuint nr_userclip:4;
- GLuint do_flat_shading:1;
+ GLuint has_flat_shading:1;
GLuint pv_first:1;
GLuint do_unfilled:1;
GLuint fill_cw:2; /* includes cull information */
@@ -173,8 +173,8 @@ void brw_clip_kill_thread(struct brw_clip_compile *c);
struct brw_reg brw_clip_plane_stride( struct brw_clip_compile *c );
struct brw_reg brw_clip_plane0_address( struct brw_clip_compile *c );
-void brw_clip_copy_colors( struct brw_clip_compile *c,
- GLuint to, GLuint from );
+void brw_clip_copy_flatshaded_attributes( struct brw_clip_compile *c,
+ GLuint to, GLuint from );
void brw_clip_init_clipmask( struct brw_clip_compile *c );
diff --git a/src/mesa/drivers/dri/i965/brw_clip_line.c
b/src/mesa/drivers/dri/i965/brw_clip_line.c
index 9ce80b8..8df7d0c 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_line.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_line.c
@@ -272,11 +272,11 @@ void brw_emit_line_clip( struct brw_clip_compile *c )
brw_clip_line_alloc_regs(c);
brw_clip_init_ff_sync(c);
- if (c-key.do_flat_shading) {
+ if (c-key.has_flat_shading) {
if (c-key.pv_first)
- brw_clip_copy_colors(c, 1, 0);
+ brw_clip_copy_flatshaded_attributes(c, 1, 0);
else
- brw_clip_copy_colors(c, 0, 1);
+ brw_clip_copy_flatshaded_attributes(c, 0, 1);
}
clip_and_emit_line(c);
diff --git a/src/mesa/drivers/dri/i965/brw_clip_tri.c
b/src/mesa/drivers/dri/i965/brw_clip_tri.c
index bea0853..ac24929 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
@@ -190,8 +190,8 @@ void brw_clip_tri_flat_shade( struct brw_clip_compile
*c )
brw_IF(p, BRW_EXECUTE_1);
{
- brw_clip_copy_colors(c, 1, 0);
- brw_clip_copy_colors(c, 2, 0);
+ brw_clip_copy_flatshaded_attributes(c, 1, 0);
+ brw_clip_copy_flatshaded_attributes(c, 2, 0);
}
brw_ELSE(p);
{
@@ -203,19 +203,19 @@ void brw_clip_tri_flat_shade( struct
brw_clip_compile *c )
Re: [Mesa-dev] [PATCH V3 4/5] i965 Gen4/5: clip: Add support for noperspective varyings
On 14 July 2013 02:39, Chris Forbes chr...@ijw.co.nz wrote:
Adds support for interpolating noperspective varyings linearly in screen
space when clipping.
Based on Olivier Galibert's patch from last year:
http://lists.freedesktop.org/archives/mesa-dev/2012-July/024341.html
At this point all -fixed and -vertex interpolation tests work.
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/brw_clip.c | 8 ++
src/mesa/drivers/dri/i965/brw_clip.h | 1 +
src/mesa/drivers/dri/i965/brw_clip_util.c | 120
+++---
3 files changed, 119 insertions(+), 10 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
b/src/mesa/drivers/dri/i965/brw_clip.c
index 7df4b18..6dd3759 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -211,6 +211,14 @@ brw_upload_clip_prog(struct brw_context *brw)
}
}
+ key.has_noperspective_shading = 0;
+ for (i = 0; i brw-vue_map_geom_out.num_slots; i++) {
+ if (key.interpolation_mode[i] == INTERP_QUALIFIER_NOPERSPECTIVE) {
+ key.has_noperspective_shading = 1;
+ break;
+ }
+ }
+
As with key.has_flat_shading, I'd prefer to see this moved to
compile_clip_prog() and brw_clip_compile.
key.pv_first = (ctx-Light.ProvokingVertex ==
GL_FIRST_VERTEX_CONVENTION);
/* _NEW_TRANSFORM (also part of VUE map)*/
key.nr_userclip = _mesa_bitcount_64(ctx-Transform.ClipPlanesEnabled);
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h
b/src/mesa/drivers/dri/i965/brw_clip.h
index 656254b..90e3e33 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -47,6 +47,7 @@ struct brw_clip_prog_key {
GLuint primitive:4;
GLuint nr_userclip:4;
GLuint has_flat_shading:1;
+ GLuint has_noperspective_shading:1;
GLuint pv_first:1;
GLuint do_unfilled:1;
GLuint fill_cw:2; /* includes cull information */
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c
b/src/mesa/drivers/dri/i965/brw_clip_util.c
index 8a21c1f..56c9ccd 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -128,6 +128,8 @@ static void brw_clip_project_vertex( struct
brw_clip_compile *c,
/* Interpolate between two vertices and put the result into a0.0.
* Increment a0.0 accordingly.
+ *
+ * Beware that dest_ptr can be equal to v0_ptr!
*/
void brw_clip_interp_vertex( struct brw_clip_compile *c,
struct brw_indirect dest_ptr,
@@ -137,7 +139,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile
*c,
bool force_edgeflag)
{
struct brw_compile *p = c-func;
- struct brw_reg tmp = get_tmp(c);
+ struct brw_reg t_nopersp, v0_ndc_copy;
GLuint slot;
/* Just copy the vertex header:
@@ -148,12 +150,107 @@ void brw_clip_interp_vertex( struct
brw_clip_compile *c,
*/
brw_copy_indirect_to_indirect(p, dest_ptr, v0_ptr, 1);
- /* Iterate over each attribute (could be done in pairs?)
+
+ /* First handle the 3D and NDC interpolation, in case we
+* need noperspective interpolation. Doing it early has no
+* performance impact in any case.
+*/
+
+ /* Take a copy of the v0 NDC coordinates, in case dest == v0. */
+ if (c-key.has_noperspective_shading) {
+ GLuint offset = brw_varying_to_offset(c-vue_map,
+ BRW_VARYING_SLOT_NDC);
+ v0_ndc_copy = get_tmp(c);
+ brw_MOV(p, v0_ndc_copy, deref_4f(v0_ptr, offset));
+ }
+
+ /* Compute the new 3D position
+*
+* dest_hpos = v0_hpos * (1 - t0) + v1_hpos * t0
+*/
+ {
+ GLuint delta = brw_varying_to_offset(c-vue_map, VARYING_SLOT_POS);
+ struct brw_reg tmp = get_tmp(c);
+ brw_MUL(p, vec4(brw_null_reg()), deref_4f(v1_ptr, delta), t0);
+ brw_MAC(p, tmp, negate(deref_4f(v0_ptr, delta)), t0);
+ brw_ADD(p, deref_4f(dest_ptr, delta), deref_4f(v0_ptr, delta), tmp);
+ release_tmp(c, tmp);
+ }
+
+ /* Recreate the projected (NDC) coordinate in the new vertex header */
+ brw_clip_project_vertex(c, dest_ptr);
+
+ /* If we have noperspective attributes,
+* we need to compute the screen-space t
+*/
+ if (c-key.has_noperspective_shading) {
+ GLuint delta = brw_varying_to_offset(c-vue_map,
+BRW_VARYING_SLOT_NDC);
+ struct brw_reg tmp = get_tmp(c);
+ t_nopersp = get_tmp(c);
+
+ /* t_nopersp = vec4(v1.xy, dest.xy) */
+ brw_MOV(p, t_nopersp, deref_4f(v1_ptr, delta));
+ brw_MOV(p, tmp, deref_4f(dest_ptr, delta));
+ brw_set_access_mode(p, BRW_ALIGN_16);
+ brw_MOV(p,
+ brw_writemask(t_nopersp, WRITEMASK_ZW),
+ brw_swizzle(tmp, 0, 1, 0, 1));
+
+
Re: [Mesa-dev] [PATCH V3 5/5] i965 Gen4/5: clip: Don't mangle flat varyings
On 14 July 2013 02:39, Chris Forbes chr...@ijw.co.nz wrote:
This patch ensures that integers will pass through unscathed. Doing
(useless) computations on them is risky, especially when their bit
patterns correspond to values like inf or nan.
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/brw_clip_util.c | 57
++-
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c
b/src/mesa/drivers/dri/i965/brw_clip_util.c
index 56c9ccd..a168b32 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -246,8 +246,8 @@ void brw_clip_interp_vertex( struct brw_clip_compile
*c,
int varying = c-vue_map.slot_to_varying[slot];
GLuint delta = brw_vue_slot_to_offset(slot);
- /* HPOS is already handled above */
- if (varying == VARYING_SLOT_POS)
+ /* HPOS, NDC already handled above */
+ if (varying == VARYING_SLOT_POS || varying == BRW_VARYING_SLOT_NDC)
continue;
Was this hunk supposed to go in patch 4/5?
With that fixed, this patch is:
Reviewed-by: Paul Berry stereotype...@gmail.com
@@ -269,28 +269,39 @@ void brw_clip_interp_vertex( struct brw_clip_compile
*c,
* header), so interpolate:
*
*New = attr0 + t*attr1 - t*attr0
+ *
+ * Unless the attribute is flat shaded -- in which case just copy
+ * from one of the sources (doesn't matter which; already copied
from pv)
*/
- struct brw_reg tmp = get_tmp(c);
- struct brw_reg t =
-c-key.interpolation_mode[slot] ==
INTERP_QUALIFIER_NOPERSPECTIVE ?
-t_nopersp : t0;
-
-brw_MUL(p,
-vec4(brw_null_reg()),
-deref_4f(v1_ptr, delta),
-t);
-
-brw_MAC(p,
-tmp,
-negate(deref_4f(v0_ptr, delta)),
-t);
-
-brw_ADD(p,
-deref_4f(dest_ptr, delta),
-deref_4f(v0_ptr, delta),
-tmp);
-
- release_tmp(c, tmp);
+ GLuint interp = c-key.interpolation_mode[slot];
+
+ if (interp != INTERP_QUALIFIER_FLAT) {
+struct brw_reg tmp = get_tmp(c);
+struct brw_reg t =
+ interp == INTERP_QUALIFIER_NOPERSPECTIVE ? t_nopersp : t0;
+
+brw_MUL(p,
+ vec4(brw_null_reg()),
+ deref_4f(v1_ptr, delta),
+ t);
+
+brw_MAC(p,
+ tmp,
+ negate(deref_4f(v0_ptr, delta)),
+ t);
+
+brw_ADD(p,
+ deref_4f(dest_ptr, delta),
+ deref_4f(v0_ptr, delta),
+ tmp);
+
+release_tmp(c, tmp);
+ }
+ else {
+brw_MOV(p,
+ deref_4f(dest_ptr, delta),
+ deref_4f(v0_ptr, delta));
+ }
}
}
--
1.8.3.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V3 5/5] i965 Gen4/5: clip: Don't mangle flat varyings
Oops, yes.
On Wed, Jul 31, 2013 at 9:45 AM, Paul Berry stereotype...@gmail.com wrote:
On 14 July 2013 02:39, Chris Forbes chr...@ijw.co.nz wrote:
This patch ensures that integers will pass through unscathed. Doing
(useless) computations on them is risky, especially when their bit
patterns correspond to values like inf or nan.
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/brw_clip_util.c | 57
++-
1 file changed, 34 insertions(+), 23 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c
b/src/mesa/drivers/dri/i965/brw_clip_util.c
index 56c9ccd..a168b32 100644
--- a/src/mesa/drivers/dri/i965/brw_clip_util.c
+++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
@@ -246,8 +246,8 @@ void brw_clip_interp_vertex( struct brw_clip_compile
*c,
int varying = c-vue_map.slot_to_varying[slot];
GLuint delta = brw_vue_slot_to_offset(slot);
- /* HPOS is already handled above */
- if (varying == VARYING_SLOT_POS)
+ /* HPOS, NDC already handled above */
+ if (varying == VARYING_SLOT_POS || varying == BRW_VARYING_SLOT_NDC)
continue;
Was this hunk supposed to go in patch 4/5?
With that fixed, this patch is:
Reviewed-by: Paul Berry stereotype...@gmail.com
@@ -269,28 +269,39 @@ void brw_clip_interp_vertex( struct brw_clip_compile
*c,
* header), so interpolate:
*
*New = attr0 + t*attr1 - t*attr0
+ *
+ * Unless the attribute is flat shaded -- in which case just
copy
+ * from one of the sources (doesn't matter which; already copied
from pv)
*/
- struct brw_reg tmp = get_tmp(c);
- struct brw_reg t =
-c-key.interpolation_mode[slot] ==
INTERP_QUALIFIER_NOPERSPECTIVE ?
-t_nopersp : t0;
-
-brw_MUL(p,
-vec4(brw_null_reg()),
-deref_4f(v1_ptr, delta),
-t);
-
-brw_MAC(p,
-tmp,
-negate(deref_4f(v0_ptr, delta)),
-t);
-
-brw_ADD(p,
-deref_4f(dest_ptr, delta),
-deref_4f(v0_ptr, delta),
-tmp);
-
- release_tmp(c, tmp);
+ GLuint interp = c-key.interpolation_mode[slot];
+
+ if (interp != INTERP_QUALIFIER_FLAT) {
+struct brw_reg tmp = get_tmp(c);
+struct brw_reg t =
+ interp == INTERP_QUALIFIER_NOPERSPECTIVE ? t_nopersp : t0;
+
+brw_MUL(p,
+ vec4(brw_null_reg()),
+ deref_4f(v1_ptr, delta),
+ t);
+
+brw_MAC(p,
+ tmp,
+ negate(deref_4f(v0_ptr, delta)),
+ t);
+
+brw_ADD(p,
+ deref_4f(dest_ptr, delta),
+ deref_4f(v0_ptr, delta),
+ tmp);
+
+release_tmp(c, tmp);
+ }
+ else {
+brw_MOV(p,
+ deref_4f(dest_ptr, delta),
+ deref_4f(v0_ptr, delta));
+ }
}
}
--
1.8.3.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V4 1/5] i965 Gen4/5: Compute interpolation status for every varying in one place.
On 23 July 2013 01:16, Chris Forbes chr...@ijw.co.nz wrote:
The program keys are updated accordingly, but the values are not used
yet.
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
V3: Updated for vue_map changes, intel - brw merge, etc. (Chris Forbes)
V4: Compute interpolation map as a new state atom rather than tacking it
on the front of the clip setup
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/Makefile.sources| 1 +
src/mesa/drivers/dri/i965/brw_clip.c | 8 ++-
src/mesa/drivers/dri/i965/brw_clip.h | 1 +
src/mesa/drivers/dri/i965/brw_context.h | 7 ++
src/mesa/drivers/dri/i965/brw_interpolation_map.c | 85
+++
src/mesa/drivers/dri/i965/brw_sf.c| 7 +-
src/mesa/drivers/dri/i965/brw_sf.h| 1 +
src/mesa/drivers/dri/i965/brw_state.h | 1 +
src/mesa/drivers/dri/i965/brw_state_upload.c | 3 +
9 files changed, 112 insertions(+), 2 deletions(-)
create mode 100644 src/mesa/drivers/dri/i965/brw_interpolation_map.c
diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
b/src/mesa/drivers/dri/i965/Makefile.sources
index 1f401fe..ac8487b 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -62,6 +62,7 @@ i965_FILES = \
brw_gs.c \
brw_gs_emit.c \
brw_gs_state.c \
+ brw_interpolation_map.c \
brw_lower_texture_gradients.cpp \
brw_misc_state.c \
brw_program.c \
diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
b/src/mesa/drivers/dri/i965/brw_clip.c
index 2ebf3f6..60d85e8 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -141,6 +141,10 @@ brw_upload_clip_prog(struct brw_context *brw)
/* Populate the key:
*/
+
+ /* BRW_NEW_INTERPOLATION_MAP */
+ memcpy(key.interpolation_mode, brw-interpolation_mode,
BRW_VARYING_SLOT_COUNT);
Random idea, which you may feel free to disregard: What if we make
interpolation_mode a struct, e.g.:
struct interpolation_mode_map
{
unsigned char mode[BRW_VARYING_SLOT_COUNT];
}
That way, this memcpy can just change to:
key.interpolation_mode = brw-interpolation_mode;
And the compiler takes care of making sure the types match and figuring out
the right number of bytes to copy.
The disadvantage, of course, is that everywhere you use
key.interpolation_mode[...] in the rest of the series you'll have to
replace that with key.interpolation_mode.mode[...], which looks pretty
silly :)
Either way, this patch is:
Reviewed-by: Paul Berry stereotype...@gmail.com
+
/* BRW_NEW_REDUCED_PRIMITIVE */
key.primitive = brw-reduced_primitive;
/* BRW_NEW_VUE_MAP_GEOM_OUT */
@@ -256,7 +260,9 @@ const struct brw_tracked_state brw_clip_prog = {
_NEW_TRANSFORM |
_NEW_POLYGON |
_NEW_BUFFERS),
- .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
+ .brw = (BRW_NEW_REDUCED_PRIMITIVE |
+BRW_NEW_VUE_MAP_GEOM_OUT |
+BRW_NEW_INTERPOLATION_MAP)
},
.emit = brw_upload_clip_prog
};
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h
b/src/mesa/drivers/dri/i965/brw_clip.h
index 02259d4..fcbe2a0 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -43,6 +43,7 @@
*/
struct brw_clip_prog_key {
GLbitfield64 attrs;
+ unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
GLuint primitive:4;
GLuint nr_userclip:4;
GLuint do_flat_shading:1;
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 86f9f71..29e522c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -154,6 +154,7 @@ enum brw_state_id {
BRW_STATE_STATS_WM,
BRW_STATE_UNIFORM_BUFFER,
BRW_STATE_META_IN_PROGRESS,
+ BRW_STATE_INTERPOLATION_MAP,
};
#define BRW_NEW_URB_FENCE (1 BRW_STATE_URB_FENCE)
@@ -186,6 +187,7 @@ enum brw_state_id {
#define BRW_NEW_STATS_WM (1 BRW_STATE_STATS_WM)
#define BRW_NEW_UNIFORM_BUFFER (1 BRW_STATE_UNIFORM_BUFFER)
#define BRW_NEW_META_IN_PROGRESS(1 BRW_STATE_META_IN_PROGRESS)
+#define BRW_NEW_INTERPOLATION_MAP (1 BRW_STATE_INTERPOLATION_MAP)
struct brw_state_flags {
/** State update flags signalled by mesa internals */
@@ -1203,6 +1205,11 @@ struct brw_context
uint32_t render_target_format[MESA_FORMAT_COUNT];
bool format_supported_as_render_target[MESA_FORMAT_COUNT];
+ /* Interpolation modes, one byte per vue slot.
+* Used Gen4/5 by the clip|sf|wm stages. Ignored on Gen6+.
+*/
+ unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
+
/* PrimitiveRestart */
struct {
bool in_progress;
diff --git
Re: [Mesa-dev] [PATCH V3 0/5] Interpolation fixes for Gen4/5
On 22 July 2013 23:04, Kenneth Graunke kenn...@whitecape.org wrote: On 07/14/2013 02:39 AM, Chris Forbes wrote: This series adds support for GLSL 1.30 / EXT_gpu_shader4's 'flat' and 'noperspective' varying interpolation qualifiers on Gen4/5. Based on Olivier Galibert's series from July 2012, with some simplifications (that series contained a number of fixes for other bugs which have been addressed in master already -- two-sided lighting, vue map inconsistencies) The interpolation qualifiers are still passed through brw-, which Eric doesn't like in the original version -- which I have some alternatives for: A couple of thoughts: The VUE map is a data structure we invented to store the slot assignments for things between the VS - GS and GS - FS. We store this as brw-vue_map_geom_out, and flag BRW_NEW_VUE_MAP_GEOM_OUT when it changes. This is done in the brw_vs state atom currently, but probably will move to brw_gs when we get real geometry shaders. Patch 1 of your series seems to introduce a new concept, almost...the interpolation mode map. The order of varyings isn't enough for the rasterization stage - we also need to know how to interpolate each of them. Perhaps interpolation_mode should be formalized as a companion to vue_map_geom_out. Possibly created in its own state atom, which would listen to BRW_NEW_VUE_MAP_GEOM_OUT and flag BRW_NEW_INTERPOLATION_MAP or such. I'm also wondering if this information could be reused when emitting 3DSTATE_SF on Gen6 and 3DSTATE_SBE on Gen7+. I don't know if it'd save a ton of code, but might help legitimize it as a data structure akin to VUE map, and keep me from thinking that this concept is only important on Gen4-5 :) Paul, do you have an opinion? I like the way the series looks now, with the interpolation mode map formalized into its own state atom, but that state atom only runs on Gen4-5 where it's needed. I'm not yet convinced that it would save us much to try to use it on Gen6-7. If anyone has the energy for it, it might be interesting to try computing the interpolation mode map in Mesa core (perhaps in do_set_program_inouts()); that way it only has to get computed once per compile, and all back-ends could potentially benefit from it. The tricky bit would be dealing with the fact that color interpolation has to get overridden based on brw-ctx.Light.ShadeModel if it's not explicitly set in the shader. Anyway, this is just me spitballing ideas for the future. I think the series looks great as is. Thanks again for your meticulous work on this, Chris. Regardless of how that turns out, I think the rest of your patches which use the new info can be reviewed now...it'll get communicated somehow. I'll try to do that soon. 1) Leave it as-is, but if anything other than the program key builders go looking in brw-interpolation_mode, it's *wrong*. 2) Tack it onto the VUE map, and have the VS code produce it at the same time as the VUE map itself. (When we have a GS, the GS will have to do this too). 3) Make the clip program key the primary source, and have the SF program key builder go snooping in it. 4) Have the clip and SF program key builders redundantly compute it. The other review point from V2 no longer exists -- fixes to the two-sided lighting behavior are already done. I've dropped all the Reviewed-by: lines from V2, since this has changed a fair bit and could do with a fresh look. With this series, all of the GLSL 1.30 interpolation tests pass on Gen5 except those which use gl_ClipDistance, since it's not supported yet. -- Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/15] mesa, glsl, st/dri: add a new driconf option force_glsl_version for Unigine
On 07/30/2013 02:11 PM, Marek Olšák wrote: Hi Kenneth, Sorry I haven't had time to test Heaven 4.0 and I'm currently using the radeonsi driver, which only supports GL 2.1, GLSL 1.30, and it isn't in good shape to run Heaven at the moment. Besides that, Heaven 4.0 throws an error (X Error of failed request: GLXBadFBConfig), while Heaven 3.0 at least starts and crashes in the driver due to lack of register allocation. That's because it asks for a 3.2 context. If you set: export MESA_GL_VERSION_OVERRIDE=3.2 export MESA_GLSL_VERSION_OVERRIDE=150 it should work. It works with a core profile, and is generally a lot less buggy. Anyway, I think we should support Heaven 3.0 despite its bugs, and that's what this patch is all about. Marek I don't see any reason to support it. As far as I can tell, you can't even download 3.0 anymore. Unigine's website(*) only has a link for 4.0. (*) http://unigine.com/products/heaven/ We don't support buggy, out of date versions of other games...once the vendor releases an update, we just tell everyone to use that. Then again, if you still want to support Tropics and Sanctuary, you'll likely need these same workarounds. *shrug* --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Should we add new error cases to stable releases?
On 07/30/2013 12:13 PM, Carl Worth wrote: Hi Ken, In the last stable-release cycle you argued for not cherry picking commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273 on the grounds that we shouldn't change mesa to be strictly more strict in a stable release. That is, adding a new error case to the compiler cannot actually cause a valid program to start working, (which would be a nice bug fix), but could potentially cause an invalid program to stop working (which could be regarded as a regression). The same reasoning seems to disqualify the below patch. Do you agree? Anyone else have input on this issue? If we agree on the rationale, I'd like to codify it with some language in our documentation for stable-branch criteria. Something like: Note: Not all bug fixes are automatically suitable for the stable branch. For example, a patch that makes the implementation more strict, (such as detecting some invalid GLSL code that was previously silently accepted), can be rejected by the stable-branch maintainer. The rationale is that a patch like this can cause a program which was previously working as desired to now stop working due to the new error case. From the point of view of the user of such an application, that's a regression that we do not want to allow during a stable release. -Carl I agree that disallowing centroid out is not something that should be cherry-picked back to 9.1, since it won't fix valid applications. My intention here was for the patch to go to 9.2, which has completely rewritten qualifier handling. Since 9.2.0 isn't out yet, it's not a risk of breaking things in a point release. It's a matter of getting bug fixes into the major release before it comes out. --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V4 1/5] i965 Gen4/5: Compute interpolation status for every varying in one place.
I like that. It also provides a sensible type to pass to the new `is
there any flat shading?` helper.
On Wed, Jul 31, 2013 at 9:52 AM, Paul Berry stereotype...@gmail.com wrote:
On 23 July 2013 01:16, Chris Forbes chr...@ijw.co.nz wrote:
The program keys are updated accordingly, but the values are not used
yet.
[V1-2]: Signed-off-by: Olivier Galibert galibert at pobox.com
V3: Updated for vue_map changes, intel - brw merge, etc. (Chris Forbes)
V4: Compute interpolation map as a new state atom rather than tacking it
on the front of the clip setup
Signed-off-by: Chris Forbes chr...@ijw.co.nz
---
src/mesa/drivers/dri/i965/Makefile.sources| 1 +
src/mesa/drivers/dri/i965/brw_clip.c | 8 ++-
src/mesa/drivers/dri/i965/brw_clip.h | 1 +
src/mesa/drivers/dri/i965/brw_context.h | 7 ++
src/mesa/drivers/dri/i965/brw_interpolation_map.c | 85
+++
src/mesa/drivers/dri/i965/brw_sf.c| 7 +-
src/mesa/drivers/dri/i965/brw_sf.h| 1 +
src/mesa/drivers/dri/i965/brw_state.h | 1 +
src/mesa/drivers/dri/i965/brw_state_upload.c | 3 +
9 files changed, 112 insertions(+), 2 deletions(-)
create mode 100644 src/mesa/drivers/dri/i965/brw_interpolation_map.c
diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
b/src/mesa/drivers/dri/i965/Makefile.sources
index 1f401fe..ac8487b 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -62,6 +62,7 @@ i965_FILES = \
brw_gs.c \
brw_gs_emit.c \
brw_gs_state.c \
+ brw_interpolation_map.c \
brw_lower_texture_gradients.cpp \
brw_misc_state.c \
brw_program.c \
diff --git a/src/mesa/drivers/dri/i965/brw_clip.c
b/src/mesa/drivers/dri/i965/brw_clip.c
index 2ebf3f6..60d85e8 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.c
+++ b/src/mesa/drivers/dri/i965/brw_clip.c
@@ -141,6 +141,10 @@ brw_upload_clip_prog(struct brw_context *brw)
/* Populate the key:
*/
+
+ /* BRW_NEW_INTERPOLATION_MAP */
+ memcpy(key.interpolation_mode, brw-interpolation_mode,
BRW_VARYING_SLOT_COUNT);
Random idea, which you may feel free to disregard: What if we make
interpolation_mode a struct, e.g.:
struct interpolation_mode_map
{
unsigned char mode[BRW_VARYING_SLOT_COUNT];
}
That way, this memcpy can just change to:
key.interpolation_mode = brw-interpolation_mode;
And the compiler takes care of making sure the types match and figuring out
the right number of bytes to copy.
The disadvantage, of course, is that everywhere you use
key.interpolation_mode[...] in the rest of the series you'll have to replace
that with key.interpolation_mode.mode[...], which looks pretty silly :)
Either way, this patch is:
Reviewed-by: Paul Berry stereotype...@gmail.com
+
/* BRW_NEW_REDUCED_PRIMITIVE */
key.primitive = brw-reduced_primitive;
/* BRW_NEW_VUE_MAP_GEOM_OUT */
@@ -256,7 +260,9 @@ const struct brw_tracked_state brw_clip_prog = {
_NEW_TRANSFORM |
_NEW_POLYGON |
_NEW_BUFFERS),
- .brw = (BRW_NEW_REDUCED_PRIMITIVE | BRW_NEW_VUE_MAP_GEOM_OUT)
+ .brw = (BRW_NEW_REDUCED_PRIMITIVE |
+BRW_NEW_VUE_MAP_GEOM_OUT |
+BRW_NEW_INTERPOLATION_MAP)
},
.emit = brw_upload_clip_prog
};
diff --git a/src/mesa/drivers/dri/i965/brw_clip.h
b/src/mesa/drivers/dri/i965/brw_clip.h
index 02259d4..fcbe2a0 100644
--- a/src/mesa/drivers/dri/i965/brw_clip.h
+++ b/src/mesa/drivers/dri/i965/brw_clip.h
@@ -43,6 +43,7 @@
*/
struct brw_clip_prog_key {
GLbitfield64 attrs;
+ unsigned char interpolation_mode[BRW_VARYING_SLOT_COUNT];
GLuint primitive:4;
GLuint nr_userclip:4;
GLuint do_flat_shading:1;
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 86f9f71..29e522c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -154,6 +154,7 @@ enum brw_state_id {
BRW_STATE_STATS_WM,
BRW_STATE_UNIFORM_BUFFER,
BRW_STATE_META_IN_PROGRESS,
+ BRW_STATE_INTERPOLATION_MAP,
};
#define BRW_NEW_URB_FENCE (1 BRW_STATE_URB_FENCE)
@@ -186,6 +187,7 @@ enum brw_state_id {
#define BRW_NEW_STATS_WM (1 BRW_STATE_STATS_WM)
#define BRW_NEW_UNIFORM_BUFFER (1 BRW_STATE_UNIFORM_BUFFER)
#define BRW_NEW_META_IN_PROGRESS(1 BRW_STATE_META_IN_PROGRESS)
+#define BRW_NEW_INTERPOLATION_MAP (1
BRW_STATE_INTERPOLATION_MAP)
struct brw_state_flags {
/** State update flags signalled by mesa internals */
@@ -1203,6 +1205,11 @@ struct brw_context
uint32_t render_target_format[MESA_FORMAT_COUNT];
bool format_supported_as_render_target[MESA_FORMAT_COUNT];
+ /* Interpolation modes, one byte per vue slot.
+* Used Gen4/5 by the
Re: [Mesa-dev] [PATCH 20/34] mesa: Validate the drawing primitive against the geometry shader input primitive type.
On 07/28/2013 11:03 PM, Paul Berry wrote:
From: Fabian Bieler fabianbie...@fastmail.fm
Reviewed-by: Paul Berry stereotype...@gmail.com
---
src/mesa/main/api_validate.c | 68
1 file changed, 68 insertions(+)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 8a2ec7b..7d4a4c1 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -245,6 +245,74 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode,
const char *name)
return GL_FALSE;
}
+ /* From the ARB_geometry_shader4 spec:
+*
+* The error INVALID_OPERATION is generated if Begin, or any command that
+* implicitly calls Begin, is called when a geometry shader is active and:
+*
+* * the input primitive type of the current geometry shader is
+* POINTS and mode is not POINTS,
+*
+* * the input primitive type of the current geometry shader is
+* LINES and mode is not LINES, LINE_STRIP, or LINE_LOOP,
+*
+* * the input primitive type of the current geometry shader is
+* TRIANGLES and mode is not TRIANGLES, TRIANGLE_STRIP or
+* TRIANGLE_FAN,
+*
+* * the input primitive type of the current geometry shader is
+* LINES_ADJACENCY_ARB and mode is not LINES_ADJACENCY_ARB or
+* LINE_STRIP_ADJACENCY_ARB, or
+*
+* * the input primitive type of the current geometry shader is
+* TRIANGLES_ADJACENCY_ARB and mode is not
+* TRIANGLES_ADJACENCY_ARB or TRIANGLE_STRIP_ADJACENCY_ARB.
+*
+ */
+ if(ctx-Shader.CurrentGeometryProgram) {
Style nit: missing space. should be:
if (ctx-Shader.CurrentGeometryProgram) {
+ const GLenum geom_mode =
+ ctx-Shader.CurrentGeometryProgram-Geom.InputType;
+ switch (mode) {
+ case GL_POINTS:
+ valid_enum = (geom_mode == GL_POINTS);
+ break;
+ case GL_LINES:
+ case GL_LINE_LOOP:
+ case GL_LINE_STRIP:
+ valid_enum = (geom_mode == GL_LINES);
+ break;
+ case GL_TRIANGLES:
+ case GL_TRIANGLE_STRIP:
+ case GL_TRIANGLE_FAN:
+ valid_enum = (geom_mode == GL_TRIANGLES);
+ break;
+ case GL_QUADS:
+ case GL_QUAD_STRIP:
+ case GL_POLYGON:
+ valid_enum = false;
+ break;
+ case GL_LINES_ADJACENCY:
+ case GL_LINE_STRIP_ADJACENCY:
+ valid_enum = (geom_mode == GL_LINES_ADJACENCY);
+ break;
+ case GL_TRIANGLES_ADJACENCY:
+ case GL_TRIANGLE_STRIP_ADJACENCY:
+ valid_enum = (geom_mode == GL_TRIANGLES_ADJACENCY);
+ break;
+ default:
+ valid_enum = false;
+ break;
+ }
+ if (!valid_enum) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ %s(mode=%s vs geometry shader input %s),
+ name,
+ _mesa_lookup_prim_by_nr(mode),
+ _mesa_lookup_prim_by_nr(geom_mode));
+ return GL_FALSE;
+ }
+ }
+
/* From the GL_EXT_transform_feedback spec:
*
* The error INVALID_OPERATION is generated if Begin, or any command
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/34] main: Allow for the possibility of GL 3.2 without ARB_geometry_shader4.
On 29 July 2013 11:17, Kenneth Graunke kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
+
+ /**
+* True if the implementation supports GLSL 1.50 style geometry
shaders.
+* This boolean is distinct from gl_extensions::ARB_geometry_**shader4
so
+* that we can expose GLSL 1.50 (and GL 3.2) functionality without
exposing
+* {ARB,EXT}_geometry_shader4.
+*/
+ GLboolean GeometryShaders150;
};
I don't really like this new flag. In my mind, ctx-Const.GLSLVersion =
150 is sufficient, since I believe geometry shaders are required to expose
1.50.
ctx-Const.GLSLVersion is already used to compute the GL version.
That's a good point. The GeometryShaders150 constant is really kind of
weird, and it's hard to imagine a back-end wanting to expose GLSL 1.50
capability without geometry shaders. I'll fix this up.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/34] main: Allow for the possibility of GL 3.2 without ARB_geometry_shader4.
On 29 July 2013 11:17, Kenneth Graunke kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
+/**
+ * Checks if the context supports geometry shaders.
+ */
+static inline GLboolean
+_mesa_has_geometry_shaders(const struct gl_context *ctx)
+{
+ return _mesa_is_desktop_gl(ctx)
+ (ctx-Version = 32 || ctx-Extensions.ARB_geometry_shader4);
+}
I might change this to:
return _mesa_is_desktop_gl(ctx)
(ctx-Const.GLSLVersion = 150 || ctx-Extensions.ARB_geometry_**
shader4);
I have a minor preference for keeping this as is, since it's conceivable
that we might one day want to support GLSL 1.50 on some platforms that
don't support GL 3.2 (much as Chris Forbes is currently doing to support
GLSL 1.30 on Gen4-5). The places where _mesa_has_geometry_shaders() is
used are for enabling and disabling API functionality (e.g. determining
whether LINES_ADJACENCY is a valid primitive mode, or whether
glFramebufferTexture() is allowed to be called), and I think that in this
hypothetical platform that supports GLSL 1.50 but not GL 3.2, those pieces
of functionality should be disabled. But I admit I'm straying pretty far
into thought experiment territory at this point.
+
+
#ifdef __cplusplus
}
#endif
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index a29f1ab..f0f59a6 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2472,7 +2472,7 @@ _mesa_FramebufferTexture(**GLenum target, GLenum
attachment,
{
GET_CURRENT_CONTEXT(ctx);
- if (ctx-Version = 32 || ctx-Extensions.ARB_geometry_**shader4) {
+ if (_mesa_has_geometry_shaders(**ctx)) {
framebuffer_texture(ctx, Layer, target, attachment, 0, texture,
level, 0, GL_TRUE);
} else {
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 0b33fa4..09b008a 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -989,7 +989,7 @@ check_extra(struct gl_context *ctx, const char *func,
const struct value_desc *d
case EXTRA_EXT_UBO_GS4:
api_check = GL_TRUE;
api_found = (ctx-Extensions.ARB_uniform_**buffer_object
- ctx-Extensions.ARB_geometry_**shader4);
+ _mesa_has_geometry_shaders(**ctx));
break;
case EXTRA_END:
break;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index aba7d84..efa2d39 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2978,6 +2978,14 @@ struct gl_constants
GLint MaxColorTextureSamples;
GLint MaxDepthTextureSamples;
GLint MaxIntegerSamples;
+
+ /**
+* True if the implementation supports GLSL 1.50 style geometry
shaders.
+* This boolean is distinct from gl_extensions::ARB_geometry_**shader4
so
+* that we can expose GLSL 1.50 (and GL 3.2) functionality without
exposing
+* {ARB,EXT}_geometry_shader4.
+*/
+ GLboolean GeometryShaders150;
};
I don't really like this new flag. In my mind, ctx-Const.GLSLVersion =
150 is sufficient, since I believe geometry shaders are required to expose
1.50.
ctx-Const.GLSLVersion is already used to compute the GL version.
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 9c8af87..e8303c8 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -177,7 +177,7 @@ validate_shader_target(const struct gl_context *ctx,
GLenum type)
case GL_VERTEX_SHADER:
return ctx-Extensions.ARB_vertex_**shader;
case GL_GEOMETRY_SHADER_ARB:
- return _mesa_is_desktop_gl(ctx) ctx-Extensions.ARB_geometry_**
shader4;
+ return _mesa_has_geometry_shaders(**ctx);
default:
return false;
}
@@ -476,8 +476,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
GLenum pname, GLint *param
/* Are geometry shaders available in this context?
*/
- const bool has_gs =
- _mesa_is_desktop_gl(ctx) ctx-Extensions.ARB_geometry_**
shader4;
+ const bool has_gs = _mesa_has_geometry_shaders(**ctx);
/* Are uniform buffer objects available in this context?
*/
diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
index ab9b14c..0e836cc 100644
--- a/src/mesa/main/version.c
+++ b/src/mesa/main/version.c
@@ -262,7 +262,7 @@ compute_version(struct gl_context *ctx)
ctx-Extensions.ARB_depth_**clamp
ctx-Extensions.ARB_draw_**elements_base_vertex
ctx-Extensions.ARB_fragment_**coord_conventions
- ctx-Extensions.ARB_geometry_**shader4
Removing ARB_geometry_shader4 here makes a lot of sense to me. IMO, the
GLSLVersion = 150 check above (but not shown in the diff) is already
sufficient.
+ ctx-Const.GeometryShaders150
Re: [Mesa-dev] [PATCH 06/34] draw/gs: fix allocation of buffer for GS output vertices
On 29 July 2013 11:09, Zack Rusin za...@vmware.com wrote: That looks wrong to me. We already account for the other fields in the vertex_size. This patch came from Bryan Cain's original geometry shader patch series--I admit I'm not familiar enough with Gallium code to know how to fix it. Anyone want to step in and address Zack's comment? Or the Gallium-related comments on patches 08 and 24? If no one has time to work on Gallium geometry shaders right now, that's ok. I can pull the Gallium stuff out of this series and archive it in a branch until someone has time to pick it up. - Original Message - From: Bryan Cain bryanca...@gmail.com Before, it accounted for the size of the vertices but not the other fields in the vertex_header struct, which caused memory corruption. --- src/gallium/auxiliary/draw/draw_gs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/draw/draw_gs.c b/src/gallium/auxiliary/draw/draw_gs.c index cd63e2b..78727c6 100644 --- a/src/gallium/auxiliary/draw/draw_gs.c +++ b/src/gallium/auxiliary/draw/draw_gs.c @@ -560,7 +560,8 @@ int draw_geometry_shader_run(struct draw_geometry_shader *shader, /* we allocate exactly one extra vertex per primitive to allow the GS to emit * overflown vertices into some area where they won't harm anyone */ output_verts-verts = - (struct vertex_header *)MALLOC(output_verts-vertex_size * + (struct vertex_header *)MALLOC(sizeof(struct vertex_header) + + output_verts-vertex_size * max_out_prims * shader-primitive_boundary); -- 1.8.3.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/34] glsl: add ir_emitvertex and ir_endprim instruction types
On 30 July 2013 11:08, Kenneth Graunke kenn...@whitecape.org wrote: On 07/28/2013 11:03 PM, Paul Berry wrote: From: Bryan Cain bryanca...@gmail.com These correspond to the EmitVertex and EndPrimitive functions in GLSL. v2 (Paul Berry stereotype...@gmail.com): Add stub implementations of new pure visitor functions to i965's vec4_visitor and fs_visitor classes. I would prefer to see these called: ir_emit_vertex ir_end_primitive The GLSL constructs are called EmitVertex and EndPrimitive, and so the obvious conversion from CamelCase to lowercase_with_underscores would result in ir_emit_vertex/ir_end_**primitive. I hate to make that suggestion, since it means a lot of pointless churn, but...somehow ir_emitvertex just bothers me every time I see it. Yeah, I agree with you. Consistency is good, and now is the best time to achieve it since the series hasn't landed yet. I'll make the changes you suggest. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/34] draw/gs: fix allocation of buffer for GS output vertices
On Tue, Jul 30, 2013 at 8:46 PM, Paul Berry stereotype...@gmail.com wrote: On 29 July 2013 11:09, Zack Rusin za...@vmware.com wrote: That looks wrong to me. We already account for the other fields in the vertex_size. This patch came from Bryan Cain's original geometry shader patch series--I admit I'm not familiar enough with Gallium code to know how to fix it. Anyone want to step in and address Zack's comment? Or the Gallium-related comments on patches 08 and 24? If no one has time to work on Gallium geometry shaders right now, that's ok. I can pull the Gallium stuff out of this series and archive it in a branch until someone has time to pick it up. This patch is a year old, and I don't remember what it was supposed to fix. The Gallium geometry shader code has changed significantly in the last year, and it should be safe to leave this patch unmerged. If any problems come up as a result, they can be addressed then. - Original Message - From: Bryan Cain bryanca...@gmail.com Before, it accounted for the size of the vertices but not the other fields in the vertex_header struct, which caused memory corruption. --- src/gallium/auxiliary/draw/draw_gs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/draw/draw_gs.c b/src/gallium/auxiliary/draw/draw_gs.c index cd63e2b..78727c6 100644 --- a/src/gallium/auxiliary/draw/draw_gs.c +++ b/src/gallium/auxiliary/draw/draw_gs.c @@ -560,7 +560,8 @@ int draw_geometry_shader_run(struct draw_geometry_shader *shader, /* we allocate exactly one extra vertex per primitive to allow the GS to emit * overflown vertices into some area where they won't harm anyone */ output_verts-verts = - (struct vertex_header *)MALLOC(output_verts-vertex_size * + (struct vertex_header *)MALLOC(sizeof(struct vertex_header) + + output_verts-vertex_size * max_out_prims * shader-primitive_boundary); -- 1.8.3.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/34] glsl: add builtins for geometry shaders.
On 30 July 2013 11:13, Kenneth Graunke kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
From: Bryan Cain bryanca...@gmail.com
v2 (Paul Berry stereotype...@gmail.com): Account for rework of
builtin_variables.cpp. Use INTERP_QUALIFIER_FLAT for gl_PrimitiveID
so that it will obey provoking vertex conventions. Convert to GLSL
1.50 style geometry shaders.
---
src/glsl/builtin_variables.cpp | 11 +--
src/glsl/builtins/ir/**EmitVertex.ir | 5 +
src/glsl/builtins/ir/**EndPrimitive.ir | 5 +
src/glsl/builtins/profiles/**150.geom | 3 +++
src/glsl/builtins/tools/**generate_builtins.py | 6 --
5 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 src/glsl/builtins/ir/**EmitVertex.ir
create mode 100644 src/glsl/builtins/ir/**EndPrimitive.ir
create mode 100644 src/glsl/builtins/profiles/**150.geom
diff --git a/src/glsl/builtin_variables.**cpp
b/src/glsl/builtin_variables.**cpp
index 1e88b6a..9d927a4 100644
--- a/src/glsl/builtin_variables.**cpp
+++ b/src/glsl/builtin_variables.**cpp
@@ -686,8 +686,10 @@ builtin_variable_generator::**
generate_gs_special_vars()
* the specific case of gl_PrimitiveIDIn. So we don't need to treat
* gl_PrimitiveIDIn as an {ARB,EXT}_geometry_shader4-**only
variable.
*/
- add_input(VARYING_SLOT_**PRIMITIVE_ID, int_t, gl_PrimitiveIDIn);
- add_output(VARYING_SLOT_**PRIMITIVE_ID, int_t, gl_PrimitiveID);
+ add_input(VARYING_SLOT_**PRIMITIVE_ID, int_t, gl_PrimitiveIDIn)
+ -interpolation = INTERP_QUALIFIER_FLAT;
+ add_output(VARYING_SLOT_**PRIMITIVE_ID, int_t, gl_PrimitiveID)
+ -interpolation = INTERP_QUALIFIER_FLAT;
}
This took a moment to understand :)
Could we instead use a temporary? Something like:
ir_variable *var;
var = add_input(VARYING_SLOT_**PRIMITIVE_ID, int_t,
gl_PrimitiveIDIn);
var-interpolation = INTERP_QUALIFIER_FLAT;
var = add_input(VARYING_SLOT_**PRIMITIVE_ID, int_t, gl_PrimitiveID);
var-interpolation = INTERP_QUALIFIER_FLAT;
etc.
Sure, no problem.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 20/34] mesa: Validate the drawing primitive against the geometry shader input primitive type.
On 30 July 2013 15:41, Kenneth Graunke kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
From: Fabian Bieler fabianbie...@fastmail.fm
Reviewed-by: Paul Berry stereotype...@gmail.com
---
src/mesa/main/api_validate.c | 68 ++**
++
1 file changed, 68 insertions(+)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 8a2ec7b..7d4a4c1 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -245,6 +245,74 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum
mode, const char *name)
return GL_FALSE;
}
+ /* From the ARB_geometry_shader4 spec:
+*
+* The error INVALID_OPERATION is generated if Begin, or any command
that
+* implicitly calls Begin, is called when a geometry shader is active
and:
+*
+* * the input primitive type of the current geometry shader is
+* POINTS and mode is not POINTS,
+*
+* * the input primitive type of the current geometry shader is
+* LINES and mode is not LINES, LINE_STRIP, or LINE_LOOP,
+*
+* * the input primitive type of the current geometry shader is
+* TRIANGLES and mode is not TRIANGLES, TRIANGLE_STRIP or
+* TRIANGLE_FAN,
+*
+* * the input primitive type of the current geometry shader is
+* LINES_ADJACENCY_ARB and mode is not LINES_ADJACENCY_ARB or
+* LINE_STRIP_ADJACENCY_ARB, or
+*
+* * the input primitive type of the current geometry shader is
+* TRIANGLES_ADJACENCY_ARB and mode is not
+* TRIANGLES_ADJACENCY_ARB or TRIANGLE_STRIP_ADJACENCY_ARB.
+*
+ */
+ if(ctx-Shader.**CurrentGeometryProgram) {
Style nit: missing space. should be:
if (ctx-Shader.**CurrentGeometryProgram) {
Fixed.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: obey clarified shift behavior
From: Roland Scheidegger srol...@vmware.com llvm shifts are undefined for shift counts exceeding (or matching) bit width, so need to apply a mask for the tgsi shift instructions. v2: only use mask for the tgsi shift instructions, not for the build shift helpers. None of the internal callers need this behavior, and while llvm can optimize away the masking for constants there are legitimate cases where it might not be able to do so even if we know that shift count must be smaller than type width (currently all such callers do not use the build shift helpers). Looks good to me ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/34] main: Allow for the possibility of GL 3.2 without ARB_geometry_shader4.
On 07/30/2013 07:30 PM, Paul Berry wrote:
On 29 July 2013 11:17, Kenneth Graunke kenn...@whitecape.org
mailto:kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
+/**
+ * Checks if the context supports geometry shaders.
+ */
+static inline GLboolean
+_mesa_has_geometry_shaders(const struct gl_context *ctx)
+{
+ return _mesa_is_desktop_gl(ctx)
+ (ctx-Version = 32 || ctx-Extensions.ARB_geometry_shader4);
+}
I might change this to:
return _mesa_is_desktop_gl(ctx)
(ctx-Const.GLSLVersion = 150 ||
ctx-Extensions.ARB_geometry___shader4);
I have a minor preference for keeping this as is, since it's conceivable
that we might one day want to support GLSL 1.50 on some platforms that
don't support GL 3.2 (much as Chris Forbes is currently doing to support
GLSL 1.30 on Gen4-5). The places where _mesa_has_geometry_shaders() is
used are for enabling and disabling API functionality (e.g. determining
whether LINES_ADJACENCY is a valid primitive mode, or whether
glFramebufferTexture() is allowed to be called), and I think that in
this hypothetical platform that supports GLSL 1.50 but not GL 3.2, those
pieces of functionality should be disabled. But I admit I'm straying
pretty far into thought experiment territory at this point.
Sure, I don't really mind keeping it as is.
I personally believe that exposing GLSL 1.50 without Geometry Shaders
isn't legal, but I know not everyone thinks that way. Either way, the
GL 3.2 check better captures the API exists and should be largely
equivalent anyway.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/34] glsl: support compilation of geometry shaders
On 30 July 2013 15:16, Kenneth Graunke kenn...@whitecape.org wrote:
On 07/28/2013 11:03 PM, Paul Berry wrote:
+ /* The 'varying in' and 'varying out' qualifiers can only be used
with
+ * ARB_geometry_shader4 and EXT_geometry_shader4, which we don't
support
+ * yet.
+ */
+ if (this-type-qualifier.flags.**q.varying) {
+if (this-type-qualifier.flags.**q.inhttp://qualifier.flags.q.in)
{
+ _mesa_glsl_error( loc, state,
+`varying in' qualifier in declaration of
+`%s' only valid for geometry shaders using
+ARB_geometry_shader4 or
EXT_geometry_shader4,
+decl-identifier);
+}
+else if (this-type-qualifier.flags.**q.out) {
Style-nit:
} else if (this-type-qualifier.flagsq.**out) {
(Mesa historically has used the two-line style in your patch, but has
largely moved away from that. In particular, the compiler has always used
} else {.)
Might be nice to kill the tabs too, but not a big deal either way.
Fixed.
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index ae79a39..af9d77e 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -2112,7 +2112,7 @@ ir_has_call(ir_instruction *ir);
extern void
do_set_program_inouts(exec_**list *instructions, struct gl_program
*prog,
- bool is_fragment_shader);
+ GLenum shader_type);
This patch is getting pretty huge. It might be nice to do the
s/is_fragment_shader/shader_**type/ in a separate patch, since that's an
obvious hunk.
That's a very good idea. I've split it out to a new patch immediately
preceding this one.
@@ -112,14 +111,24 @@
ir_set_program_inouts_visitor::visit(ir_dereference_variable *ir)
return visit_continue;
if (ir-type-is_array()) {
- mark(this-prog, ir-var, 0,
- ir-type-length * ir-type-fields.array-matrix_columns,
- this-is_fragment_shader);
+ int matrix_columns = ir-type-fields.array-matrix_columns;
+ int length = ir-type-length;
I was wondering if this was left over from the ARB_gs4 stuff. Apparently
it's for lowered arrays of instance blocks? Maybe a comment would be in
order.
Good point. I'm not sure I spent enough time reviewing this part of the
commit myself, nor testing it. Tomorrow I'll try to verify that this is
necessary and correct.
@@ -129,7 +138,40 @@
ir_set_program_inouts_visitor::visit_enter(ir_dereference_array *ir)
ir_dereference_variable *deref_var;
ir_constant *index = ir-array_index-as_constant()**;
deref_var = ir-array-as_dereference_**variable();
- ir_variable *var = deref_var ? deref_var-var : NULL;
+ ir_variable *var;
+ bool is_vert_array = false, is_2D_array = false;
+
+ /* Check whether this dereference is of a GS input array. These are
special
+* because the array index refers to the index of an input vertex
instead of
+* the attribute index. The exceptions to this exception are 2D
arrays
+* such as gl_TexCoordIn. For these, there is a nested
dereference_array,
+* where the inner index specifies the vertex and the outer index
specifies
+* the attribute. To complicate things further, matrix columns are
also
+* accessed with dereference_array. So we have to correctly handle 1D
+* arrays of non-matrices, 1D arrays of matrices, 2D arrays of
non-matrices,
+* and 2D arrays of matrices.
+*/
+ if (this-shader_type == GL_GEOMETRY_SHADER) {
+ if (!deref_var) {
+ /* Either an outer (attribute) dereference of a 2D array or a
column
+ * dereference of an array of matrices. */
Style-nit: */ goes on a separate line.
Fixed.
+ ir_dereference_array *inner_deref = ir-array-as_dereference_*
*array();
+ assert(inner_deref);
+ deref_var = inner_deref-array-as_**dereference_variable();
+ is_2D_array = true;
+ }
+
+ if (deref_var deref_var-var-mode == ir_var_shader_in) {
+ if (ir-type-is_array())
+/* Inner (vertex) dereference of a 2D array */
+return visit_continue;
+ else
+/* Dereference of a 1D (vertex) array */
+is_vert_array = true;
+ }
+ }
I'm not terribly comfortable with this code, but I'm not sure what to
suggest instead. One concern I have is varying structs.
As I understand it, if a VS outputs a struct, the corresponding GS input
would be an array of structs. Wouldn't those be accessed via (array_ref
(record_ref ...) ...)?
The code above seems to assume that if it's not
(array_ref (var_ref ...))
then it *must* be
(array_ref (array_ref ...) ...)
which seems wrong, or at least dubious.
The this must be a matrix stuff concerns me too.
Good point. I'll look at this in more detail too.
@@ -174,6 +177,77 @@ private:
};
+class
