Review: displacement bug fix (issue1897048)


larry...@...
 

Reviewers: ,

Description:
Yet another fix to the P & N copying code for displacement to make the
interpreter and LLVM work the same way -- don't "find" the symbol from a
layer that never was bound.


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

Affected files:
src/liboslexec/context.cpp
src/liboslexec/oslexec_pvt.h


Index: src/liboslexec/oslexec_pvt.h
===================================================================
--- src/liboslexec/oslexec_pvt.h (revision 790)
+++ src/liboslexec/oslexec_pvt.h (working copy)
@@ -1203,6 +1203,10 @@
/// user-data attached
bool renderer_has_userdata (ustring name, TypeDesc type, void *renderstate);

+ /// Has the layer been bound?
+ ///
+ bool bound () const { return m_bound; }
+
/// Has this layer already executed?
///
bool executed () const { return m_executed; }
Index: src/liboslexec/context.cpp
===================================================================
--- src/liboslexec/context.cpp (revision 790)
+++ src/liboslexec/context.cpp (working copy)
@@ -383,7 +383,7 @@
ASSERT((size_t)nlayers <= m_exec[use].size());
for (int layer = nlayers-1; layer >= 0; --layer) {
ShadingExecution &exec (m_exec[use][layer]);
- if (exec.instance()) {
+ if (exec.instance() && exec.bound()) {
Symbol *sym = exec.symbol (name);
if (sym)
return sym;


cliffo...@...
 

On 2010/08/02 18:19:05, larrygritz wrote:


LGTM

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


larry...@...
 

Actually, it's even easier. Instead of

if (exec.instance() && exec.bound()) {

it's ok to just test

if (exec.bound()) {

Bound implies that there is a good instance pointer, not the other way
around necessarily. That was my mistake all along.


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


cliffo...@...
 

On 2010/08/02 18:41:45, larrygritz wrote:
Actually, it's even easier. Instead of
if (exec.instance() && exec.bound()) {
it's ok to just test
if (exec.bound()) {
Bound implies that there is a good instance pointer, not the other way
around
necessarily. That was my mistake all along.
LGTM

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