Reviewers: ,

Description:
Missed a couple:

* Two spots where we asked the renderer for stuff and asked for derivs
anytime it was float-based, instead of when the destination symbol had
them.

* Always mark P as needing derivs if it's written in a shader, we need
that for displacement to work right.

* Fix a bad ASSERT in aref constant folding (too restrictive, since we
allow constants to be strings and matrices these days too).


Please review this at http://codereview.appspot.com/895041/show

Affected files:
src/liboslexec/exec.cpp
src/liboslexec/opattribute.cpp
src/liboslexec/runtimeoptimize.cpp


Index: src/liboslexec/exec.cpp
===================================================================
--- src/liboslexec/exec.cpp (revision 615)
+++ src/liboslexec/exec.cpp (working copy)
@@ -415,10 +415,9 @@
adjust_varying(sym, true, false /* don't keep old values */);
m_conditional_level = old_conditional_level;

- bool wants_derivatives = (sym.typespec().is_float() || sym.typespec().is_triple());
ShaderGlobals *globals = m_context->m_globals;
// FIXME: runflags required here even if we are using something else
- if (!get_renderer_userdata(m_runstate_stack.front().runflags, m_npoints, wants_derivatives,
+ if (!get_renderer_userdata(m_runstate_stack.front().runflags, m_npoints, sym.has_derivs(),
sym.name(), sym.typespec().simpletype(),
&globals->renderstate[0], globals->renderstate.step(),
sym.data(), sym.step())) {
Index: src/liboslexec/opattribute.cpp
===================================================================
--- src/liboslexec/opattribute.cpp (revision 615)
+++ src/liboslexec/opattribute.cpp (working copy)
@@ -107,17 +107,15 @@
attribute_type = Destination.typespec().simpletype();

// always fully varying case
- bool derivatives = (Destination.typespec().is_float() || Destination.typespec().is_triple());
-
SHADE_LOOP_BEGIN
result[i] = array_lookup ?
exec->get_renderer_array_attribute(globals->renderstate[i],
- derivatives /* want derivatives */,
+ Destination.has_derivs(),
object_lookup ? object_name[i] : ustring(),
attribute_type, attribute_name[i],
index[i], &destination[i]) :
exec->get_renderer_attribute(globals->renderstate[i],
- derivatives /* want derivatives */,
+ Destination.has_derivs(),
object_lookup ? object_name[i] : ustring(),
attribute_type, attribute_name[i],
&destination[i]);
Index: src/liboslexec/runtimeoptimize.cpp
===================================================================
--- src/liboslexec/runtimeoptimize.cpp (revision 615)
+++ src/liboslexec/runtimeoptimize.cpp (working copy)
@@ -793,7 +793,6 @@
DASSERT (A.typespec().is_array() && Index.typespec().is_int());
if (A.is_constant() && Index.is_constant()) {
TypeSpec elemtype = A.typespec().elementtype();
- ASSERT (elemtype.is_float() || elemtype.is_triple() || elemtype.is_int());
ASSERT (equivalent(elemtype, R.typespec()));
int index = *(int *)Index.data();
DASSERT (index < A.typespec().arraylength());
@@ -1641,6 +1640,11 @@
}
}

+ // Writing to P needs derivs
+ Symbol *Psym = inst.symbol (inst.findsymbol (Strings::P));
+ if (Psym && Psym->symtype() == SymTypeGlobal && Psym->everused())
+ Psym->has_derivs (true);
+
// Propagate derivative dependencies for any syms already known to
// need derivs. It's probably marked that way because another layer
// downstream connects to it and needs derivatives of that

LGTM

http://codereview.appspot.com/895041/show