Date   

Re: Review: nan/inf checking (issue194142)

Larry Gritz <l...@...>
 

You mean checking all the inputs? Sure. But let me check this one in first.


On Jan 29, 2010, at 12:20 PM, <cku...@...> wrote:

How about checking the results of geometry binding as well?

Otherwise LGTM

http://codereview.appspot.com/194142/show
--
Larry Gritz
l...@...


Re: Review: nan/inf checking (issue194142)

cku...@...
 

How about checking the results of geometry binding as well?

Otherwise LGTM

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


Review: nan/inf checking (issue194142)

larry...@...
 

Reviewers: ,

Description:
This change allows a debug mode (set by attribute("debugnan",1)) that
checks every value written by each instruction, and if any nan or inf
values are found, issues a warning pointing to exactly which shader
source line it happened on. It's quite expensive, nobody should use it
for ordinary rendering, but it's real helpful if you are trying to debug
NaNs in your renderer, this will quickly narrow it down to the
particular instruction.

(SPI folks: a corresponding renderer-side review will come to you
shortly.)

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

Affected files:
src/liboslexec/exec.cpp
src/liboslexec/oslexec_pvt.h
src/liboslexec/shadingsys.cpp


Re: Review: compiler failed to mark a const as "not written" (issue195084)

Larry Gritz <l...@...>
 

Right before checking in, I noticed that the same mistake could happen for ordinary loops:

while (1) { }
or
do { } while(0);

I just fixed in in both places in the identical way.


On Jan 28, 2010, at 8:50 AM, <cliffo...@...> <cliffo...@...> wrote:

On this one, I'll have to take your word for it. LGTM!

http://codereview.appspot.com/195084/show
--
Larry Gritz
l...@...


Re: Compiling OpenShadingLanguage under Windows

Oleg <ode...@...>
 

Hi Jeremy,

Sure, you can do it. It would be great.

Oleg

On 28 Jan., 00:23, Wormszer <wo...@...> wrote:
Mcpp integrated pretty easily with the existing structure. I can send  
you the project I made for it. Unfortunatly I wasn't able to get it to  
build on vs with cpp syntax. I think it assumed some other things  
would be around that are installed with gcc.

I was thinking I might send you a patch first and you can apply it to  
a clean checkout and see if you hit any issues. Then I can submit it  
to the osl dev.

I think we are a minority on windows and it might be worth trying it  
on a second windows machine before saying it works.

I will be back at home tomorrow and will send you the project file.

You could look into the print precision issue I mentioned. I'm hoping  
that's just a simple fix and possibly the last issue.

Jeremy

Sent from my iPhone

On Jan 27, 2010, at 3:48 PM, Oleg <od...@...> wrote:

Hi Jeremy,
I was trying to use the boost.wave preprocessor, but still have some
troubles to integrate it. I was not able to run any tests so far.
However, I'm still trying :-) Maybe it would be useful if you submit
the project file for the mcpp as well.
Regards,
Oleg
On 27 Jan., 06:41, Wormszer <wo...@...> wrote:
I just checked one failed test file output and the error was
ref:
-5.96046e-08
out:
-5.96046e-008
I wonder if this is due to different default precision on cout  
possibly?
That was the only error so I am not sure.
Any ideas?
I guess the printing is being done in the shader, so maybe ill look  
there at
the printf operator. It was the only value printed using scientific
notation.
I imagine the others will probably be something similiar.
Jeremy
On Wed, Jan 27, 2010 at 12:31 AM, Wormszer <wo...@...>  
wrote:
I have made some modifications to runtest.py to allow for the  
tests to be
run on windows along with some changes to the cmake files.
I have redone the everything from scratch and have made changes to  
cmake
and the source that should make it easy to build on windows.
I have some extra instructions for setting up some of the  
dependencies and
then modified the cmake routines to find the external libraries as  
they come
from installing and building OIIO.
OIIO cmake routines still need manual help and I may go modify them,
instead of having to add a bunch of paths to CMAKE_INSTALL_PREFIX.
I can now run the tests as well but only 35% of them pass.
I was hoping that through including the flex and bison outputs in  
the
project the need for cygwin could be removed, but the testsuite  
uses diff so
it maybe here to stay.
To get around having to modify every test, and having to parse the  
command
lines to pull out the output file names and create a stdout to  
file pipe, i
decieded to write the command to a batch file and have python call  
that.
This worked except that my install folder is C:\program files  
(x86)\... and
of course the space was now giving me issues.
So i was able to parse the command strings and wrap them commands  
in quotes
and then write them to the batch file and run it.
Now of course that wasn't the end of it. The next issue is that  
windows
uses cr/lf i guess through > even though the output should be  
writing just
lf, so diff was puking on that.
So on windows the diff command has to take an extra cmd parameter
--strip-trailing-cr
So now I can run the tests, I will investigate the errors and then  
should
be ready to submit a patch.
Jeremy
--
You received this message because you are subscribed to the Google  
Groups "OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to osl...@...
.
For more options, visit this group athttp://groups.google.com/group/osl-dev?hl=en
.


Re: Review: compiler failed to mark a const as "not written" (issue195084)

cliffo...@...
 

On this one, I'll have to take your word for it. LGTM!

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


Re: Missing sqrtf in beckman's sample function (issue196043)

cku...@...
 


Missing sqrtf in beckman's sample function (issue196043)

aco...@...
 

Reviewers: osl-dev_googlegroups.com,

Description:
There was a problem with the sample function in beckman closure. A
missing sqrt for sampling the tangent of theta. It was preventing
reflections from getting the right blur. This patch fixes the problem.

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

Affected files:
src/liboslexec/bsdf_microfacet.cpp


Index: src/liboslexec/bsdf_microfacet.cpp
===================================================================
--- src/liboslexec/bsdf_microfacet.cpp (revision 551)
+++ src/liboslexec/bsdf_microfacet.cpp (working copy)
@@ -374,7 +374,7 @@ public:
// we take advantage of cos(atan(x)) == 1/sqrt(1+x^2)
// and sin(atan(x)) == x/sqrt(1+x^2)
float alpha2 = m_ab * m_ab;
- float tanThetaM = -alpha2 * logf(1 - randu);
+ float tanThetaM = sqrtf(-alpha2 * logf(1 - randu));
float cosThetaM = 1 / sqrtf(1 + tanThetaM * tanThetaM);
float sinThetaM = cosThetaM * tanThetaM;
float phiM = 2 * float(M_PI) * randv;


Review: compiler failed to mark a const as "not written" (issue195084)

larry...@...
 

Reviewers: ,

Description:
Bug fix:

When a function's 'return' statement is not the last statement in the
function body, we use a trick where we wrap the function in a 'do { ...
} while (0)' and the return is like a 'break'. (This is because we
don't have a true jump instruction, which is a pain for SIMD.) We say
'while 0' because we don't want to LOOP, we just want a loop to jump out
of, if you catch my drift.

The bug is that I made the "0" a true constant, but forgot to mark that
the while condition was not written by the while statement itself, and
therefore got a "can't write to a constant" error later on.


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

Affected files:
src/liboslcomp/codegen.cpp


Index: src/liboslcomp/codegen.cpp
===================================================================
--- src/liboslcomp/codegen.cpp (revision 552)
+++ src/liboslcomp/codegen.cpp (working copy)
@@ -1181,7 +1181,8 @@
Symbol *condvar = m_compiler->make_constant (0);
size_t argstart = m_compiler->add_op_args (1, &condvar);
m_compiler->ircode(loop_op).set_args (argstart, 1);
- m_compiler->ircode(loop_op).argread (0, true); // read also
+ m_compiler->ircode(loop_op).argread (0, true); // read
+ m_compiler->ircode(loop_op).argwrite (0, false); // not written
int endlabel = m_compiler->next_op_label ();
m_compiler->ircode(loop_op).set_jump (startlabel, startlabel,
endlabel, endlabel);


Re: Review: smarter runflags for lazy eval (issue194117)

larry...@...
 

On 2010/01/27 23:22:32, ckulla wrote:
Can't wait to see the difference on those benchmark cases.
OK, I committed it, so give it a try.
This should throw into even more stark relief the waste associated with
sparse runflags. I'll tackle that after "rebind" is done.


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


Re: Compiling OpenShadingLanguage under Windows

Wormszer <worm...@...>
 

Mcpp integrated pretty easily with the existing structure. I can send you the project I made for it. Unfortunatly I wasn't able to get it to build on vs with cpp syntax. I think it assumed some other things would be around that are installed with gcc.

I was thinking I might send you a patch first and you can apply it to a clean checkout and see if you hit any issues. Then I can submit it to the osl dev.

I think we are a minority on windows and it might be worth trying it on a second windows machine before saying it works.

I will be back at home tomorrow and will send you the project file.

You could look into the print precision issue I mentioned. I'm hoping that's just a simple fix and possibly the last issue.

Jeremy

On Jan 27, 2010, at 3:48 PM, Oleg <ode...@...> wrote:

Hi Jeremy,

I was trying to use the boost.wave preprocessor, but still have some
troubles to integrate it. I was not able to run any tests so far.
However, I'm still trying :-) Maybe it would be useful if you submit
the project file for the mcpp as well.

Regards,
Oleg

On 27 Jan., 06:41, Wormszer <wo...@...> wrote:
I just checked one failed test file output and the error was
ref:
-5.96046e-08
out:
-5.96046e-008
I wonder if this is due to different default precision on cout possibly?
That was the only error so I am not sure.

Any ideas?

I guess the printing is being done in the shader, so maybe ill look there at
the printf operator. It was the only value printed using scientific
notation.

I imagine the others will probably be something similiar.

Jeremy

On Wed, Jan 27, 2010 at 12:31 AM, Wormszer <wo...@...> wrote:
I have made some modifications to runtest.py to allow for the tests to be
run on windows along with some changes to the cmake files.
I have redone the everything from scratch and have made changes to cmake
and the source that should make it easy to build on windows.
I have some extra instructions for setting up some of the dependencies and
then modified the cmake routines to find the external libraries as they come
from installing and building OIIO.
OIIO cmake routines still need manual help and I may go modify them,
instead of having to add a bunch of paths to CMAKE_INSTALL_PREFIX.
I can now run the tests as well but only 35% of them pass.
I was hoping that through including the flex and bison outputs in the
project the need for cygwin could be removed, but the testsuite uses diff so
it maybe here to stay.
To get around having to modify every test, and having to parse the command
lines to pull out the output file names and create a stdout to file pipe, i
decieded to write the command to a batch file and have python call that.
This worked except that my install folder is C:\program files (x86)\... and
of course the space was now giving me issues.
So i was able to parse the command strings and wrap them commands in quotes
and then write them to the batch file and run it.
Now of course that wasn't the end of it. The next issue is that windows
uses cr/lf i guess through > even though the output should be writing just
lf, so diff was puking on that.
So on windows the diff command has to take an extra cmd parameter
--strip-trailing-cr
So now I can run the tests, I will investigate the errors and then should
be ready to submit a patch.
Jeremy
--
You received this message because you are subscribed to the Google Groups "OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to osl...@... .
For more options, visit this group at http://groups.google.com/group/osl-dev?hl=en .


Re: Review: smarter runflags for lazy eval (issue194117)

cku...@...
 

LGTM2

Can't wait to see the difference on those benchmark cases.

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


Re: Review: smarter runflags for lazy eval (issue194117)

aco...@...
 


Review: smarter runflags for lazy eval (issue194117)

larry...@...
 

Reviewers: ,

Description:
If you called context->execute() with a "sparse" set of runflags (such
as if you were picking the subset of a set of rays that hit objects with
the same shaders), it would use those runflags for any
unconditionally-run layers, but lazy layer evaluation was done with all
points on, which is very wasteful for sparse run sets. This change
records the top-level original runflags passed in, and uses those
runflags when running upstream layers lazily.

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

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


Re: Compiling OpenShadingLanguage under Windows

Oleg <ode...@...>
 

Hi Jeremy,

I was trying to use the boost.wave preprocessor, but still have some
troubles to integrate it. I was not able to run any tests so far.
However, I'm still trying :-) Maybe it would be useful if you submit
the project file for the mcpp as well.

Regards,
Oleg

On 27 Jan., 06:41, Wormszer <wo...@...> wrote:
I just checked one failed test file output and the error was
ref:
-5.96046e-08
out:
-5.96046e-008
I wonder if this is due to different default precision on cout possibly?
That was the only error so I am not sure.

Any ideas?

I guess the printing is being done in the shader, so maybe ill look there at
the printf operator. It was the only value printed using scientific
notation.

I imagine the others will probably be something similiar.

Jeremy

On Wed, Jan 27, 2010 at 12:31 AM, Wormszer <wo...@...> wrote:
I have made some modifications to runtest.py to allow for the tests to be
run on windows along with some changes to the cmake files.
I have redone the everything from scratch and have made changes to cmake
and the source that should make it easy to build on windows.
I have some extra instructions for setting up some of the dependencies and
then modified the cmake routines to find the external libraries as they come
from installing and building OIIO.
OIIO cmake routines still need manual help and I may go modify them,
instead of having to add a bunch of paths to CMAKE_INSTALL_PREFIX.
I can now run the tests as well but only 35% of them pass.
I was hoping that through including the flex and bison outputs in the
project the need for cygwin could be removed, but the testsuite uses diff so
it maybe here to stay.
To get around having to modify every test, and having to parse the command
lines to pull out the output file names and create a stdout to file pipe, i
decieded to write the command to a batch file and have python call that.
This worked except that my install folder is C:\program files (x86)\... and
of course the space was now giving me issues.
So i was able to parse the command strings and wrap them commands in quotes
and then write them to the batch file and run it.
Now of course that wasn't the end of it. The next issue is that windows
uses cr/lf i guess through > even though the output should be writing just
lf, so diff was puking on that.
So on windows the diff command has to take an extra cmd parameter
--strip-trailing-cr
So now I can run the tests, I will investigate the errors and then should
be ready to submit a patch.
Jeremy


Re: Compiling OpenShadingLanguage under Windows

Wormszer <worm...@...>
 

I just checked one failed test file output and the error was
ref:
-5.96046e-08
out: 
-5.96046e-008
I wonder if this is due to different default precision on cout possibly? That was the only error so I am not sure.
 
Any ideas?
 
I guess the printing is being done in the shader, so maybe ill look there at the printf operator. It was the only value printed using scientific notation.
 
I imagine the others will probably be something similiar.
 
Jeremy

 

On Wed, Jan 27, 2010 at 12:31 AM, Wormszer <worm...@...> wrote:
I have made some modifications to runtest.py to allow for the tests to be run on windows along with some changes to the cmake files.

I have redone the everything from scratch and have made changes to cmake and the source that should make it easy to build on windows.
I have some extra instructions for setting up some of the dependencies and then modified the cmake routines to find the external libraries as they come from installing and building OIIO.
OIIO cmake routines still need manual help and I may go modify them, instead of having to add a bunch of paths to CMAKE_INSTALL_PREFIX.
 
I can now run the tests as well but only 35% of them pass.
 
I was hoping that through including the flex and bison outputs in the project the need for cygwin could be removed, but the testsuite uses diff so it maybe here to stay.
 
To get around having to modify every test, and having to parse the command lines to pull out the output file names and create a stdout to file pipe, i decieded to write the command to a batch file and have python call that.
This worked except that my install folder is C:\program files (x86)\... and of course the space was now giving me issues.
 
So i was able to parse the command strings and wrap them commands in quotes and then write them to the batch file and run it.
 
Now of course that wasn't the end of it. The next issue is that windows uses cr/lf i guess through > even though the output should be writing just lf, so diff was puking on that.
So on windows the diff command has to take an extra cmd parameter --strip-trailing-cr
 
So now I can run the tests, I will investigate the errors and then should be ready to submit a patch.
 
Jeremy

 
On Mon, Jan 25, 2010 at 12:48 PM, Wormszer <worm...@...> wrote:
I have been getting things ready for a patch, but now I am stuck on the testsuite.

There seems to be some issues with python and the os.system command not liking spaces in file names.
So a newer method i think is to use subprocess.call, but this doesn't seem to recognize the redirection operator > and it gets passed as a command line arg instead of directing stdout to a file.

The fix for this I believe is to use the subprocess call but instead of > use its options to pass in a file to stdout.

I have been working on parsing the commands to break out the > and determine file names.

Does anyone know if all the tests use out.txt for the output, and the second command appends to it? I haven't had a chance to look at them all yet.
I am trying to find a way with just modifying the main runtest definition.

If I can get them running then I will feel better about submitting a patch that should work.

Jeremy



On Fri, Jan 22, 2010 at 11:50 PM, Wormszer <worm...@...> wrote:
I finally have it all building on windows and can run the testshade program.

I had to create some new OSL_DLLPUBLIC defines and place them around, OSLCOMP_DLLPUBLIC, OSLEXEC_DLLPUBLIC

liboslcomp exports some of the base types, and common functions. Basically if oslcomp uses it, then it exports it and anything that was duplicated in oslexec now imports it from oslcomp.
liboslexec uses the objects exported from oslcomp, typespec and a few others.
liboslquery now has dependencies on oslexec, oslcomp.

I don't believe this will affect the GCC build since they are defined to nothing, other than the source has more OSLCOMP_DLLPUBLIC, OSLEXEC_DLLPUBLIC in it.
I did move typespec.cpp to liboslcomp since its used there first.

Due to the way testshader is it makes use of oslexec private includes and I had to basically export a bunch of classes in oslexec private and public headers.
I guess for a test program this is ok, but to me it seems like its breaking the rules, by using classes in oslexec_pvt.h, and if they are indeed public should be moved to the public header
PVT does mean private right? :).

I am probably going to redo everything I did and then submit a patch. If anyone else has found any issues or a better solution for some of the issues in this thread please let me know.


Jeremy


On Fri, Jan 22, 2010 at 12:15 PM, Wormszer <worm...@...> wrote:
I have been trying to get the shaders test to build but I have run into lots of issues with linking dependencies, exports imports etc.

The projects cmake generates for example oslquery uses source files from olsexec. This is causing issues with the DLL_PUBLIC etc type of declerations, because other projects use oslexec as a library to import.
I guess this may work fine on GCC but windows is pitches a fit, because in one project the defines are correct import/export in the other they are import when they should be nothing or export.

Are the source files being duplicated in the projects on purpose? Is this just a cmake issue? 
Is this just an issue with VS because GCC doesn;t care and just links everything together?

I think i should be able to rearrange the import/export types for windows, and then make olsquery have a depencency to oslexec instead of rebuilding the lexer etc. And for any of the other projects(i think the others are better).
And make sure the projects use the library imports rather than building the code themselves?

Or should i get them to build duplicating the code in areas and just fix the special cases with more defines.

I prefer the first option, and at first i could probably isolate it to windows in cmake and the source. But maybe its a issue for gcc as well in some areas.
Or maybe oslquery needs to run standalone you dont want to inlcude a dll/so.

Any thoughts?

Jeremy



On Thu, Jan 21, 2010 at 12:39 AM, Wormszer <worm...@...> wrote:

I have mcpp integrated into the compiler now. Working like CPP and writing everything to stdout.

A few things, the binary available for download at least on windows doesn't support forcing an include file.
So i had to build it from source, luckily it wasn't to difficult to create a new project for it.

I built it as VS for VS. Now that I think about it I bet I could build it as VS for GCC so that it would support the same command line options.

Another weird issue was I guess the lexer doesn't support  c-style /* comments? These might not be defined in the osl language, and if so probably should be removed from the stdosl.h.
Its the only difference i could see, that could of been causing the error.

CL would remove the */ */ comments at the end of the math defines in stdosl.h even though i told it to preserve comments.

mcpp when i told it to leave comments would actually leave them in there(seems like correct behavior), and then the lexer would crash.

I am not sure why comments are needed really at this point, and i think i must of included them for my own debugging, i don't think CPP was set to output them.

Thanks for the suggestions on a easier solution to the pre-processor issue.

I looked real quick for the boost but mcpp seemed easier since i could just get a binary and set it in my path. (but then i had to rebuild it)

Well now to see if I can get anything to happen with my compiled shaders.

Jeremy



On Wed, Jan 20, 2010 at 9:03 PM, Wormszer <worm...@...> wrote:
I am fine with either one. I think having something embedded or buildable would be usefull.

Otherwise there maybe issues with different compilers and would probably need some kind of config or something that cmake would generate at least on windows with several versions of VS etc.

Will just have to see how Larry or the other devs feel about using one of those two for Linux builds as well. I would assume it would be wise to have all the preprocessing done with the same tool when possible.

I will look at both real quick but I might lean towards mcpp.


On Jan 20, 2010, at 8:14 PM, Chris Foster <chri...@...> wrote:

On Thu, Jan 21, 2010 at 11:02 AM, Blair Zajac <bl...@...> wrote:
The main annoyance with wave is that it causes the compiler to issue truly
horrendous error messages if you get things wrong (the wave internals make
heavy use of template metaprogramming).

(Obviously this is only a problem when integrating wave into the project
source, and that's not really difficult at all.)

There's mcpp which is designed to be an embeddable C-preprocessor.

Ice, which we use at Sony Imageworks for all our middle-tier systems, uses
mcpp for it's own internal IDL type language, so I would recommend that.

mcpp looks nice.  In some sense, using wave would mean one less dependency
since OSL already relies on boost, but it does mean linking to libboost-wave,
so if you have a modular boost install the point may be moot...

~Chris

PS: Sorry to Blair for the duplicate message.  I intended to send it
to the list :-(
--
You received this message because you are subscribed to the Google Groups "OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to osl...@....
For more options, visit this group at http://groups.google.com/group/osl-dev?hl=en.









Re: Compiling OpenShadingLanguage under Windows

Wormszer <worm...@...>
 

I have made some modifications to runtest.py to allow for the tests to be run on windows along with some changes to the cmake files.

I have redone the everything from scratch and have made changes to cmake and the source that should make it easy to build on windows.
I have some extra instructions for setting up some of the dependencies and then modified the cmake routines to find the external libraries as they come from installing and building OIIO.
OIIO cmake routines still need manual help and I may go modify them, instead of having to add a bunch of paths to CMAKE_INSTALL_PREFIX.
 
I can now run the tests as well but only 35% of them pass.
 
I was hoping that through including the flex and bison outputs in the project the need for cygwin could be removed, but the testsuite uses diff so it maybe here to stay.
 
To get around having to modify every test, and having to parse the command lines to pull out the output file names and create a stdout to file pipe, i decieded to write the command to a batch file and have python call that.
This worked except that my install folder is C:\program files (x86)\... and of course the space was now giving me issues.
 
So i was able to parse the command strings and wrap them commands in quotes and then write them to the batch file and run it.
 
Now of course that wasn't the end of it. The next issue is that windows uses cr/lf i guess through > even though the output should be writing just lf, so diff was puking on that.
So on windows the diff command has to take an extra cmd parameter --strip-trailing-cr
 
So now I can run the tests, I will investigate the errors and then should be ready to submit a patch.
 
Jeremy

 

On Mon, Jan 25, 2010 at 12:48 PM, Wormszer <worm...@...> wrote:
I have been getting things ready for a patch, but now I am stuck on the testsuite.

There seems to be some issues with python and the os.system command not liking spaces in file names.
So a newer method i think is to use subprocess.call, but this doesn't seem to recognize the redirection operator > and it gets passed as a command line arg instead of directing stdout to a file.

The fix for this I believe is to use the subprocess call but instead of > use its options to pass in a file to stdout.

I have been working on parsing the commands to break out the > and determine file names.

Does anyone know if all the tests use out.txt for the output, and the second command appends to it? I haven't had a chance to look at them all yet.
I am trying to find a way with just modifying the main runtest definition.

If I can get them running then I will feel better about submitting a patch that should work.

Jeremy



On Fri, Jan 22, 2010 at 11:50 PM, Wormszer <worm...@...> wrote:
I finally have it all building on windows and can run the testshade program.

I had to create some new OSL_DLLPUBLIC defines and place them around, OSLCOMP_DLLPUBLIC, OSLEXEC_DLLPUBLIC

liboslcomp exports some of the base types, and common functions. Basically if oslcomp uses it, then it exports it and anything that was duplicated in oslexec now imports it from oslcomp.
liboslexec uses the objects exported from oslcomp, typespec and a few others.
liboslquery now has dependencies on oslexec, oslcomp.

I don't believe this will affect the GCC build since they are defined to nothing, other than the source has more OSLCOMP_DLLPUBLIC, OSLEXEC_DLLPUBLIC in it.
I did move typespec.cpp to liboslcomp since its used there first.

Due to the way testshader is it makes use of oslexec private includes and I had to basically export a bunch of classes in oslexec private and public headers.
I guess for a test program this is ok, but to me it seems like its breaking the rules, by using classes in oslexec_pvt.h, and if they are indeed public should be moved to the public header
PVT does mean private right? :).

I am probably going to redo everything I did and then submit a patch. If anyone else has found any issues or a better solution for some of the issues in this thread please let me know.


Jeremy


On Fri, Jan 22, 2010 at 12:15 PM, Wormszer <worm...@...> wrote:
I have been trying to get the shaders test to build but I have run into lots of issues with linking dependencies, exports imports etc.

The projects cmake generates for example oslquery uses source files from olsexec. This is causing issues with the DLL_PUBLIC etc type of declerations, because other projects use oslexec as a library to import.
I guess this may work fine on GCC but windows is pitches a fit, because in one project the defines are correct import/export in the other they are import when they should be nothing or export.

Are the source files being duplicated in the projects on purpose? Is this just a cmake issue? 
Is this just an issue with VS because GCC doesn;t care and just links everything together?

I think i should be able to rearrange the import/export types for windows, and then make olsquery have a depencency to oslexec instead of rebuilding the lexer etc. And for any of the other projects(i think the others are better).
And make sure the projects use the library imports rather than building the code themselves?

Or should i get them to build duplicating the code in areas and just fix the special cases with more defines.

I prefer the first option, and at first i could probably isolate it to windows in cmake and the source. But maybe its a issue for gcc as well in some areas.
Or maybe oslquery needs to run standalone you dont want to inlcude a dll/so.

Any thoughts?

Jeremy



On Thu, Jan 21, 2010 at 12:39 AM, Wormszer <worm...@...> wrote:

I have mcpp integrated into the compiler now. Working like CPP and writing everything to stdout.

A few things, the binary available for download at least on windows doesn't support forcing an include file.
So i had to build it from source, luckily it wasn't to difficult to create a new project for it.

I built it as VS for VS. Now that I think about it I bet I could build it as VS for GCC so that it would support the same command line options.

Another weird issue was I guess the lexer doesn't support  c-style /* comments? These might not be defined in the osl language, and if so probably should be removed from the stdosl.h.
Its the only difference i could see, that could of been causing the error.

CL would remove the */ */ comments at the end of the math defines in stdosl.h even though i told it to preserve comments.

mcpp when i told it to leave comments would actually leave them in there(seems like correct behavior), and then the lexer would crash.

I am not sure why comments are needed really at this point, and i think i must of included them for my own debugging, i don't think CPP was set to output them.

Thanks for the suggestions on a easier solution to the pre-processor issue.

I looked real quick for the boost but mcpp seemed easier since i could just get a binary and set it in my path. (but then i had to rebuild it)

Well now to see if I can get anything to happen with my compiled shaders.

Jeremy



On Wed, Jan 20, 2010 at 9:03 PM, Wormszer <worm...@...> wrote:
I am fine with either one. I think having something embedded or buildable would be usefull.

Otherwise there maybe issues with different compilers and would probably need some kind of config or something that cmake would generate at least on windows with several versions of VS etc.

Will just have to see how Larry or the other devs feel about using one of those two for Linux builds as well. I would assume it would be wise to have all the preprocessing done with the same tool when possible.

I will look at both real quick but I might lean towards mcpp.


On Jan 20, 2010, at 8:14 PM, Chris Foster <chri...@...> wrote:

On Thu, Jan 21, 2010 at 11:02 AM, Blair Zajac <bl...@...> wrote:
The main annoyance with wave is that it causes the compiler to issue truly
horrendous error messages if you get things wrong (the wave internals make
heavy use of template metaprogramming).

(Obviously this is only a problem when integrating wave into the project
source, and that's not really difficult at all.)

There's mcpp which is designed to be an embeddable C-preprocessor.

Ice, which we use at Sony Imageworks for all our middle-tier systems, uses
mcpp for it's own internal IDL type language, so I would recommend that.

mcpp looks nice.  In some sense, using wave would mean one less dependency
since OSL already relies on boost, but it does mean linking to libboost-wave,
so if you have a modular boost install the point may be moot...

~Chris

PS: Sorry to Blair for the duplicate message.  I intended to send it
to the list :-(
--
You received this message because you are subscribed to the Google Groups "OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to osl...@....
For more options, visit this group at http://groups.google.com/group/osl-dev?hl=en.








Re: Review: bind refactor (issue195042)

larry...@...
 

http://codereview.appspot.com/195042/diff/1/5
File src/liboslexec/exec.cpp (right):

http://codereview.appspot.com/195042/diff/1/5#newcode191
src/liboslexec/exec.cpp:191: VaryingRef<float> * valref = NULL, *dxref =
NULL, *dyref = NULL;
On 2010/01/26 19:57:25, ckulla wrote:
This is a big confusing - they all get turned into a VaryingRef to
floats even
though they all have different types. Maybe a comment could explain
why this
isn't important here.
The idea is that the VaryingRef is really just a pointer underneath, so
I'm combining the triple and float cases rather than have two sets of
variables.
I'll put in a comment and re-submit.

http://codereview.appspot.com/195042/diff/1/5#newcode310
src/liboslexec/exec.cpp:310: // Uninitialized strings in the heap can
really screw
On 2010/01/26 19:57:25, ckulla wrote:
Could we mark this as temporary? I really think it would be better to
guarantee
this on the compiler side.
I'm not sure I agree. Why clutter up the instruction stream with
initializations? It's only a runtime implementation issue that the way
we store strings can lead to bad things for uninitialized memory in the
string variable. If this weren't the case (as it may not be for a
different "back end"), we wouldn't bother with this, necessarily.

But I'll put in a comment here that at least points out the possibility
that it could be solved in the compiler.

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


Re: Review: bind refactor (issue195042)

cku...@...
 

LGTM - just a few small notes


http://codereview.appspot.com/195042/diff/1/5
File src/liboslexec/exec.cpp (right):

http://codereview.appspot.com/195042/diff/1/5#newcode191
src/liboslexec/exec.cpp:191: VaryingRef<float> * valref = NULL, *dxref =
NULL, *dyref = NULL;
This is a big confusing - they all get turned into a VaryingRef to
floats even though they all have different types. Maybe a comment could
explain why this isn't important here.

http://codereview.appspot.com/195042/diff/1/5#newcode310
src/liboslexec/exec.cpp:310: // Uninitialized strings in the heap can
really screw
Could we mark this as temporary? I really think it would be better to
guarantee this on the compiler side.

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


Review: bind refactor (issue195042)

larry...@...
 

Reviewers: ,

Description:
1. Refactor "bind" to simplify the handling of globals.
2. Bug fix (exec.cpp:309) -- zero out local strings, the one place where
uninitialized values in the heap can crash the renderer.
3. Infrastructure for eventual "rebind" optimization, including tracking
the statistics on it and allowing an option to enable/disable.


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

Affected files:
src/liboslexec/context.cpp
src/liboslexec/exec.cpp
src/liboslexec/oslexec_pvt.h
src/liboslexec/shadingsys.cpp
src/testshade/testshade.cpp

4861 - 4880 of 5013