Appleseed inclusion in Windows version

[ericmehl]

I'm finally about ready to submit pull requests for Dependencies, Cortex and Gaffer (yay!) and I have a couple things I want to raise for discussion before submitting those.

The big one is Appleseed is not working on Windows. It crashes with Windows giving a very unhelpful error message just saying that it has encountered an error. I haven't had a chance to build in debug mode to really dive into it yet. This is with a build leaving out the shaders @boberfly and @est77 flagged in https://github.com/GafferHQ/gaffer/pull/2570 so it's already somewhat of a substandard build. It also prevents any OSL shaders to be added to a graph.

We can get into another discussion of more detailed debugging, but in short it's succeeding in all of the builds (with Maya shaders omitted) but failing in actual use. So it's ready for any interested party to debug.

I'm wondering if you consider leaving out Appleseed support to be an acceptable compromise at the time or if that is a must-have. I would guess (assuming no major issues arise in the PRs, which is defintely not guaranteed) Windows Gaffer can be ready in the next week or two without Appleseed vs. unknown with it.

[danieldresser-ie]

I'm wondering if you consider leaving out Appleseed support to be an acceptable compromise.

Sounds reasonable to both @andrewkaufman and I ( hopefully John will chime in too ). It would be great to have something working - even better if we could get it hooked up to some sort of automated testing on Windows, so that we would be warned right away if we accidently break the Windows build in the future. We don't need Appleseed for that.

In terms of getting Appleseed actually working, I'm somewhat hopeful that @est77 might be able to offer some help - I think he fairly regularly runs Appleseed on Windows.

When you say:

It also prevents any OSL shaders to be added to a graph.

This is if you include the broken Appleseed build? I would assume that if you just left out Appleseed, it should be fine to add OSL shaders, ( for example, for use in OSLObject or OSLImage ).

[johnhaddon]

Nice one Eric!

I'm wondering if you consider leaving out Appleseed support to be an acceptable compromise at the time or if that is a must-have. I would guess (assuming no major issues arise in the PRs, which is defintely not guaranteed) Windows Gaffer can be ready in the next week or two without Appleseed vs. unknown with it.

I certainly don't see the Appleseed problem as being a blocker for merging your work; just getting to the point where everything builds out of master represents huge progress. I would want to have feature parity before we called the Windows builds "official" though...

I think I'm right in saying that Appleseed sees a lot of development/use on Windows, so I'm hoping it's the sort of thing that can get resolved fairly quickly with the right eyes on it...

[ericmehl]

Great! That was my biggest question. I'm sorting out some final things, testing the latest with Travis through my repository and such. Still some tests that are failing but I'm working through them. I'll do a final rebase to clean up the details when I have them all sorted.

If @est77 could help it would be fantastic. I'm a little wary of getting sucked into a long debugging task since I don't know anything of the Appleseed code. But I'm happy to jump in where possible.

I got a first start on building on AppVeyor this weekend. It looks promising - similar to Travis where it can use a config file in the repository so it's portable across forks.

[ericmehl]

Regarding the OSL / Appleseed issue - having the Appleseed directory existing causes any OSL shader (including non-Appleseed) to fail. But not the OSLImage / OSLObject. So it's like there's some hook that is created when Gaffer starts and finds Appleseed that attempts to validate OSL shaders against Appleseed or something similar?

Here's the scenarios I experimented with:

• Default install - Appleseed directory and code + Appleseed environment variable set = crash creating OSL Shader nodes
• Appleseed code present but Appleseed removed from environment variables (so no Appleseed in the node menu) = crash creating OSL shader nodes
• Deleting the Appleseed directory = successful OSL working throughout

I haven't looked into it, but I guessed there was something where rendering engines register with Gaffer to get updates to shaders and this is causing the crash when creating non-Appleseed shaders. Is that correct? If not maybe there is some other mixup that is causing problems.

[boberfly]

From what I remember the moment Appleseed hits its init it crashes inside somewhere, looking at the debugger last time I checked. @ericmehl have a look at this commit, it might fix the shaders with huge static array LUTs: https://github.com/imageworks/OpenShadingLanguage/issues/723

Great work @ericmehl btw. @danieldresser-ie I do have a build setup which can work in a linux docker using wine/msvc for automation, but appveyor is probably best...

[est77]

We are happy to help with windows issues. Most of appleseed development happens on Windows, so it's pretty well supported and up to date.

The shaders with large arrays (btw, they are gone from master since today) are only used in Maya. Skipping them should be fine.

Would it be possible to upload your build somewhere for us to test?

[ericmehl]

That would be wonderful, thanks for having a look. I uploaded a recent build the dropbox here: https://www.dropbox.com/s/0nh9ggkh9t26m8d/Gaffer_Windows_Gaffer_0.47.0.0.249_artifacts.zip?dl=0

You need to runner gaffer_env.bat and then gaffer.bat, both from the "bin" directory. Here are my current repositories I'm building from:

• Dependencies (including Appleseed): https://github.com/hypothetical-inc/gafferDependencies/tree/msvc2017_for_merge
• Cortex (including IECoreAppleseed): https://github.com/hypothetical-inc/cortex/tree/msvc2017_for_merge
• Gaffer: https://github.com/hypothetical-inc/gaffer/tree/msvc2017

For now it's better to not merge from those - I will still be rebasing occasionally and pushing to GitHub is unfortunately the best way I know of to get them into my internal CI system to test build on both Windows and Linux. So those branches may get wiped and pushed fresh now and then.

[est77]

I checked quickly and the crash happens when trying to render the preview of any OSL shader. It does not seem related to OSL or to Gaffer. It looks like there is something wrong with the appleseed build.

Trying to use appleseed.cli outside of Gaffer to render simple scenes, even ones without any OSL shader also crashes. We'll keep looking into it.

[est77]

Unrelated to appleseed, I had some issues with the file browser and I couldn't load the gafferbot cache. I don't know if it is a known issue.

[ericmehl]

Do you mean you can't load the gafferbot at all, or it loads (showing the bounding box) and then crashes?

I'm working on some changes to how file browsers handle paths since they are different in Windows vs. Linux.

Currently you have to make sure all of the directory separators are forward slashes and there is no forward slash at the very start of the path. Currently the file browser tends to return something like "/C:\dir\dir2/resources/gafferbot.scc" and it should look like "C:/dir/dir2/resources/gafferbot.scc". I'm about halfway through sorting that out.

I'm aware of a crash when trying to display the mesh for the entire gafferbot at once. I'm able to show some pieces but others will crash it.

Your crashes with OSL and Appleseed sound like the same behavior I am getting. If you remove Appleseed and AppleseedDisplay, does your OSL work?

[johnhaddon]

I'm working on some changes to how file browsers handle paths since they are different in Windows vs. Linux.

It would be good to coordinate a bit on that - could you let me know what you're planning?

[ericmehl]

Of course - I wanted to wrap my mind around the code a little before kicking off the discussion. I have a basic familiarity now so it seems as good a time as any.

I'm not sure how familiar you are with Windows paths, skip over this paragraph if it's nothing new. Windows uses back slashes as path separators instead of Linux's forward slashes. It can also use forward slashes when specifying the path with a drive letter, but not for UNC paths. UNC paths take the form of \\server\share\dir1\file.ext and drive letter paths look like M:\dir\file.ext. UNC paths are common because they are universal across computers. The other form using a drive letter like M:\dir\file.ext could mount the M drive to different places from one computer to another. Drive letter paths also can't start with any slash, \M:\dir\file.ext will fail.

From a user point of view, most Windows people are going to expect the back slash but I don't think it's a huge ask to have them adopt forward slashes. Nuke uses forward slashes for all file paths on Windows for example. Even for UNC paths which I guess they convert to back slashes internally when actually reading and writing data.

Maybe the best way to go is to use FileSystemPath or something similar more universally? I noticed bookmarks for one use strings instead of FileSystemPath.

I have done some quick experiments but after thinking it through more it seems like this approach would lead to a collection of band-aids instead of an organized solution. I'm definitely open to advice though.

My current thought is to settle on using forward slashes internally and for storing paths in Gaffer files, and using FileSystemPath objects throughout. FileSystemPath would return paths to the UI and internally to the OS in system-specific formats. What do you think of that?

Moving everything to FileSystemPath would also be a good head start on another item I expect to crop up in the mid-future - path mapping or path substitutions. This will be required to locate files on Linux in Gaffer scripts created on Windows and vice versa. Probably a discussion for another day, but something that's in the back of my mind.

[est77]

Your crashes with OSL and Appleseed sound like the same behavior I am getting. If you remove Appleseed and AppleseedDisplay, does your OSL work?

I didn't need to remove it. If you pin the viewer to some node, the shader preview scene is not created or rendered and the crash does not happen.

I don't think it is OSL or the display plugin. I did some testing with the appleseed.cli.exe in your Gaffer build, running from outside of Gaffer in a cmd shell. I can run the unit tests, but I cannot render anything using it. Not even the cornell box scene.

[boberfly]

@ericmehl For serialisation I'd say keep unix-style for sure, best practice in a pipeline sharing between the two OSes would most likely have a base environment variable that points to the OS-specific path eg. /data/whatever.scc & Z:/data/whatever.scc would be like ${ROOT_DATA}/whatever.scc but that would be up to the user to set that up, and default would just use the Z:/ I guess...

@est77 not sure if this helps but it did crash the moment Appleseed initialises. The biggest difference I can think of with a regular Appleseed build vs this Gaffer one is that the dependencies are all dynamically linked, not statically linked. I didn't run with debug symbols before due to OSL not building in debug mode last time.

Cheers

[johnhaddon]

I'm not sure how familiar you are with Windows paths

I know nothing! I'm beginning to think I should get myself a Windows box just so I can follow along...

From a user point of view, most Windows people are going to expect the back slash but I don't think it's a huge ask to have them adopt forward slashes. Nuke uses forward slashes for all file paths on Windows for example.

This sounds good to me.

Maybe the best way to go is to use FileSystemPath or something similar more universally? I noticed bookmarks for one use strings instead of FileSystemPath.

I'm a bit wary of pushing FileSystemPath too far through the codebase. The main purpose of the Path classes was to provide an abstract model for querying/navigating a hierarchy, so we could reuse the same UI widgets with FileSystemPath and ScenePath and IndexedIOPath and so on. In lower-level spots we don't need that - we just need the filename. I'm also seriously considering refactoring the Path classes so they only perform queries, and don't have their own storage for a current path internally (in places like the HierarchyView, we store a lot of ScenePaths, and there's more overhead than I'd like).

So, ideally I think FileSystemPath would be something we only used in the UI code, and perhaps other spots where we need to navigate hieararchy. I don't know if this is a good approach or not though - I'm learning as we go here.

I have done some quick experiments but after thinking it through more it seems like this approach would lead to a collection of band-aids instead of an organized solution.

Yeah, if we want to standardise on / and not \ then it does feel like it'd be good to have a standard way of doing that rather than lots of little tweaks. I'm just not sure what that should be. On the C++ side we have boost::filesystem, which I believe provides some facilities for conversion. And then we have our own FileSystemPath, which we only really use where we need a file browser or other widget. And then there's the python standard library which is convenient and most Python developers will instinctively reach for, but which as far as I can tell, will prefer to use \ on Windows?

best practice in a pipeline sharing between the two OSes would most likely have a base environment variable that points to the OS-specific path eg. /data/whatever.scc & Z:/data/whatever.scc would be like ${ROOT_DATA}/whatever.scc but that would be up to the user to set that up, and default would just use the Z:/ I guess...

I do like the sound of that. Perhaps we can prioritise tickets like https://github.com/GafferHQ/gaffer/issues/2661, to improve the experience of working with variables in paths, rather than having to poke explicit path mapping functionality down into every single file access...

[est77]

@est77 not sure if this helps but it did crash the moment Appleseed initialises.

We are still looking into it. Windows it not very helpful when you have issues like this one.

I didn't run with debug symbols before due to OSL not building in debug mode last time

What I usually do when I have to work on windows and cannot make debug builds are release builds with debug symbols. It is not perfect, but avoids the incompatibilities when mixing debug and release code and you can actually get some information from them when you crash or have to debug something.

[boberfly]

@est77 actually I think this is how I discovered that it did crash in initialisation by running a RelWithDebugInfo build, my memory is hazy but all the meaningful information that would've appeared was in a _DEBUG pre-processor define... :)

[ericmehl]

Yeah, if we want to standardise on / and not \ then it does feel like it'd be good to have a standard way of doing that rather than lots of little tweaks. I'm just not sure what that should be. On the C++ side we have boost::filesystem, which I believe provides some facilities for conversion. And then we have our own FileSystemPath, which we only really use where we need a file browser or other widget. And then there's the python standard library which is convenient and most Python developers will instinctively reach for, but which as far as I can tell, will prefer to use \ on Windows?

boost::filesystem does have nice functions for handling cross platform paths. I gather they do a version of what we are talking about and have an internal representation and customize the return values per OS. The best explanation I found is on their tutorial page towards the bottom under "Class path: Generic format vs. Native format". If you ask Boost for a string(), c_str() or native() it will give you OS specific results (\ on Windows, / on others). If you ask for generic_string() it will give you / always.

Python's os.path.sep will give \ on Windows and / on others and I agree it would likely be the go-to for Python people.

I lean towards the idea of storing all paths in an object of some kind but defer to your judgement. I like it because it would help sanitize paths going both in and out. If someone puts a path into the object in Windows format it could be converted to generic format without the developer having to keep it straight. It could also facilitate the variable expansion from #2661?

The alternative that comes to mind is to pass strings though a function that does separator fixing and variable expansion. Would there be a performance problem here with processing large sequences through such a function? Or it could be done once on the encoded sequence path (not sure of the terminology - I mean the version with ### in the filename before it's expanded to frame numbers)?

I do like the sound of that. Perhaps we can prioritise tickets like #2661, to improve the experience of working with variables in paths, rather than having to poke explicit path mapping functionality down into every single file access...

I like that approach too with the added feature request of automatically converting paths a user types in or selects through a dialog to the variable format if it matches a known path stored in an environment variable. I picture users (including myself) forgetting to have the environment variable in all needed paths and getting failures on the render farm because it was left out.

I think it might still be nice to have a generic path substitution system based simply on string substitution. I like that it allows users to work in their native format as much as possible and let a TD / admin type of role manage the details of how paths are handled.

It should be fine to just do one pass of substitutions when a script is opened? That would correct paths on render nodes and be done. For interactive sessions it would correct paths and then the user's configuration would take care of things as they go.

My tests on Travis are passing for Cortex and Gaffer now so we're one step closer!

[johnhaddon]

I lean towards the idea of storing all paths in an object of some kind but defer to your judgement.

I wouldn't defer just yet! At the very least I'm already convinced that you've thought about this more than I have. But you might find I drag my feet on committing to major architectural changes until I'm convinced they're worth the upheaval. That's not to say I don't want the user experience to be good on Windows - I just prefer to move slowly when on unfamiliar ground...

I like it because it would help sanitize paths going both in and out. If someone puts a path into the object in Windows format it could be converted to generic format without the developer having to keep it straight.

Yeah, I see the benefits, and I agree we need some path sanitisation of some sort. I guess I'm partly wondering what we mean by "someone puts a path in" though. Is that someone always operating through the UI, or do we want to do these sanitisations on strings passed via the API?

It could also facilitate the variable expansion from #2661?

Just to clarify, the variable expansion being talked about there is something that is applied to all strings in Gaffer, so happens to also apply to filenames. The substitutions are applied at the very last minute inside the compute, and that ticket is about making the file browser savvy to that process. But my initial naive hope is that we can keep that mechanism as something generic that just applies to all strings, and keep any \->/ substitutions out of it.

with the added feature request of automatically converting paths a user types in or selects through a dialog to the variable format if it matches a known path stored in an environment variable.

Nice idea - if we funnel the right information around to get #2661 to work in the first place, I don't see any reason why this couldn't be done too.

It should be fine to just do one pass of substitutions when a script is opened?

That's a little tricky currently, because we don't know the difference between a string that happens to have a \ in it, and a filename that needs converting.

My tests on Travis are passing for Cortex and Gaffer now so we're one step closer!

Nice!

[boberfly]

+1 on the file dialog knowing about env variables to known paths and auto-correcting, you wouldn't believe how much time and effort has gone into maintaining script fixers in Maya in the past for me between another studio who didn't care for it....! (well, you can't stop C:/Users/ThoseCertainSomeones/Desktop/myTexture_final_final.png from being pathed sometimes)

[danieldresser-ie]

@ericmehl - it sounds like you're talking about doing path conversion very early in processing - trying to draw the boundary somewhere right near where the user enters paths in the UI?

I don't fully understand all the Windows stuff, but I would have thought that the most consistent place to convert file paths would be right before any operation that actually accesses the disk. Keep all paths in Gaffer with forward slashes, basically right up until actually opening the file handle?

That would keep any logic that deals with files as similar as possible.

Then we just have to make any file path choosers return paths with forward slashes, and teach the users to use forward slashes in Gaffer ( VFX users would hopefully be somewhat used to the idea of using forward slashes, thanks to tools like Nuke ).

Anyway, I maybe don't fully understand this issue, but that would be my first instinct.

[ericmehl]

I wouldn't defer just yet! At the very least I'm already convinced that you've thought about this more than I have. But you might find I drag my feet on committing to major architectural changes until I'm convinced they're worth the upheaval. That's not to say I don't want the user experience to be good on Windows - I just prefer to move slowly when on unfamiliar ground...

I've had the pleasure of thinking about path conversion much more than I'd like as a result of using a mixed platform render farm!

Just to clarify, the variable expansion being talked about there is something that is applied to all strings in Gaffer, so happens to also apply to filenames. The substitutions are applied at the very last minute inside the compute, and that ticket is about making the file browser savvy to that process. But my initial naive hope is that we can keep that mechanism as something generic that just applies to all strings, and keep any ->/ substitutions out of it.

That makes sense to me now. It does look like there needs to be a way of designating certain strings as file paths. Since Gaffer scripts are stored as Python, what if we just serialize paths wrapped in a Python function that does all the needed conversions? So something like __children["SceneReader"]["fileName"].setValue( '/path/to/cow.scc' ) would become __children["SceneReader"]["fileName"].setValue( Gaffer.SanitizePath( '/path/to/cow.scc' ) ) I don't know what repercussions that may have elsewhere though.

I don't fully understand all the Windows stuff, but I would have thought that the most consistent place to convert file paths would be right before any operation that actually accesses the disk. Keep all paths in Gaffer with forward slashes, basically right up until actually opening the file handle?

@danieldresser I think we're on the same page actually, I like the idea of keeping paths generic (forward slash) until the last minute. Having users use forward slashes seems fine too, though technically I believe Nuke will convert \ to / when you type in a path manually (not at my workstation right now, but I'll verify that this afternoon). But that's mostly a user education / convenience thing that I will (naively) say now is not a big problem.

The concern that led me to suggest also filtering paths input to Gaffer is based on what happens when a path comes from a source other than the file browser. For example, someone writing a plugin would naturally use glob or other Python methods that will return results in the OS native format. Presumably most external sources would be OS native also, like database entries, helper text files listing paths, external program return values, etc.

So either the creator of the plugin will have to ensure they sanitize all of their paths to Gaffer's standard, or it has to happen on the Gaffer side. I tilt towards doing it on the Gaffer side since it's closer to the core of the relevant code, but I'm not dead set on it.

That also speaks to John's question of

I guess I'm partly wondering what we mean by "someone puts a path in" though. In other words, that "someone" could be the user via the file browser - straightforward - or a source that is out of our control but still needs to cooperate.

Does that make sense? Maybe I'm just trying to solve a problem that doesn't really exist in practice?

[danieldresser-ie]

The concern that led me to suggest also filtering paths input to Gaffer is based on what happens when a path comes from a source other than the file browser.

Hmm, I'm probably not the most useful person to comment, since when I consider the possibility of an asset management system that returns paths with backslashes in them, my instinctive reaction is just to recoil in disgust ... but yeah, I guess it makes sense that such things exist on Windows.

So either the creator of the plugin will have to ensure they sanitize all of their paths to Gaffer's standard, or it has to happen on the Gaffer side. I tilt towards doing it on the Gaffer side since it's closer to the core of the relevant code, but I'm not dead set on it.

I guess the advantage of forcing it to be done by the plugin is that it would allow us a clear standard: "Gaffer paths always use forward slashes, if you are writing a plugin, you have to convert them before passing them to Gaffer". Possibly a bit more work for plugin authors, but at least it's consistent.

If we're saying that we would ask users to enter paths with forward slashes, but sometimes the paths could come from a plugin that uses backslashes, then something would have to accept in either format and automagically convert if it notices a backslash and the platform is Windows? This feels like it makes things a bit muddier.

I don't think John is going to like this at all, but just to explore possibilities - technically, it would be possible to add a new automatic substitution type Context::Substitutions::FilePlatformSubstitutions - if this flag was set, I guess that could be used to autoconvert backslashes on StringPlugs if the platform is Windows. The problem is that this would clash with the existing Substitutions::EscapeSubstitutions flag, which triggers handling backslashes as escape characters, and I'm not sure how we'd sort that out.

[johnhaddon]

I'm not sure if I'm getting the right end of the stick, so before I reply to the specific comments above, let me outline the fundamentals of what I think we're angling towards :

1. Always using forward slashes for Gaffer's internal representation of paths, just as we do now. This raises the question "how do we convert from back slashes on input?"
2. When opening files (ImageReader, SceneReader etc) on Windows, automatically converting from our internal forward slashes to backslashes as required. This raises the question "where do we convert to backslashes during compute?".

Is that correct?

It does look like there needs to be a way of designating certain strings as file paths. Since Gaffer scripts are stored as Python, what if we just serialize paths wrapped in a Python function that does all the needed conversions? So something like

__children["SceneReader"]["fileName"].setValue( Gaffer.SanitizePath( '/path/to/cow.scc' ) )

If we've already sanitised paths on input, then we don't need to do anything during serialisation do we? We just output them as they are?

The concern that led me to suggest also filtering paths input to Gaffer is based on what happens when a path comes from a source other than the file browser. For example, someone writing a plugin would naturally use glob or other Python methods that will return results in the OS native format. Presumably most external sources would be OS native also, like database entries, helper text files listing paths, external program return values, etc.

If I understand correctly, in a pure Windows setup, this shouldn't be an issue? If we passed the backslashes naively through Gaffer and then to open() (or whatever), that'd work fine. The problem would be if we created paths containing backslashes on Windows and then sent them to a Linux farm? Just trying to make sure I understand the problem...

If we do want magic filtering on input, then we'd need a way of knowing which StringPlugs are used for files. Then plug.setValue( someUnsanitisedPath ) could sanitise the path automatically on the way in. That would address item 1 above somewhat.

I don't think John is going to like this at all, but just to explore possibilities - technically, it would be possible to add a new automatic substitution type Context::Substitutions::FilePlatformSubstitutions - if this flag was set, I guess that could be used to autoconvert backslashes on StringPlugs if the platform is Windows. The problem is that this would clash with the existing Substitutions::EscapeSubstitutions flag, which triggers handling backslashes as escape characters, and I'm not sure how we'd sort that out.

So this would be addressing item 2, right? How to convert back to windows form before opening a file in compute()? In that case we'd be autoconverting forward slashes to back slashes wouldn't we?

But regardless, our use of backslash as an escape character is incompatible with the storage of Windows native paths internally in Gaffer, isn't it? So that's a good argument for needing item 1.

Doing the automatic conversions we're discussing doesn't seem too hard for things like SceneReader and ImageReader, where it's really clear cut that they store filenames. But there are trickier cases that are actually really common, like using a string attribute to store texture filenames, and then querying that from a shader. In that case, we may or may not know they're filenames on input, and we're also not in control of how they'll be used in the renderer. What would we want to happen there? Should we identify the input string as a filename on the CustomAttributes node that creates it, so we convert to forward slashes for storage? But then convert to the backslash form when inserting the attribute into the scene itself? And what do we do when the attribute is inside a cache file that's out of our control? Punt?

Just to throw one more spanner in the works : what about Unicode? @ericmehl, we don't really handle this properly at all right now, and have an open ticket about it. The plan is to use UTF8 internally on the C++ side, and unicode objects in Python. I believe some Windows APIs will require us to convert from UTF8 to some other form before use?

[ericmehl]

Always using forward slashes for Gaffer's internal representation of paths, just as we do now. This raises the question "how do we convert from back slashes on input?" When opening files (ImageReader, SceneReader etc) on Windows, automatically converting from our internal forward slashes to backslashes as required. This raises the question "where do we convert to backslashes during compute?".

Is that correct?

Those are the two questions on my mind.

If we've already sanitised paths on input, then we don't need to do anything during serialisation do we? We just output them as they are?

Yes, with paths sanitized on input this should be unneeded. I was thinking wrapping a path in a function would be a convenient way of doing one (or both) of two things:

1. designating the string is actually a file path and flagging it to the file reader that it should be treated as such throughout Gaffer (so it would get the backslash swap treatment as needed) or
2. support path mapping, slash swapping and variable substitution at load time only instead of doing string substitutions during compute. For example swapping out "M:\project1\file.ext" or "${CACHE_DIR}/project1/file.ext" for "/mnt/render_cache/project1/file.ext" on Linux render nodes.

Perhaps point 2 is a PR for another day.

If I understand correctly, in a pure Windows setup, this shouldn't be an issue? If we passed the backslashes naively through Gaffer and then to open() (or whatever), that'd work fine. The problem would be if we created paths containing backslashes on Windows and then sent them to a Linux farm? Just trying to make sure I understand the problem...

If we do want magic filtering on input, then we'd need a way of knowing which StringPlugs are used for files. Then plug.setValue( someUnsanitisedPath ) could sanitise the path automatically on the way in. That would address item 1 above somewhat.

That's my thinking and concern exactly. Speaking for myself (with no idea of how many studios would be looking at the same problem) my workstations are Windows and the farm is Linux. Anecdotally I know Thinkbox's Deadline render queue has good support for this kind of workflow and I've seen forum posts in various places that suggest there is a small-medium number of studios that would need this functionality. Cloud rendering, for example, is far less expensive on Linux than Windows and makes this kind of feature attractive.

That said, it might be a case of me customizing my fork of Gaffer for our use and seeing how many fellow Windows users run into this problem on the GafferHQ build and addressing it when needed.

I don't think John is going to like this at all, but just to explore possibilities - technically, it would be possible to add a new automatic substitution type Context::Substitutions::FilePlatformSubstitutions - if this flag was set, I guess that could be used to autoconvert backslashes on StringPlugs if the platform is Windows. The problem is that this would clash with the existing Substitutions::EscapeSubstitutions flag, which triggers handling backslashes as escape characters, and I'm not sure how we'd sort that out.

So this would be addressing item 2, right? How to convert back to windows form before opening a file in compute()? In that case we'd be autoconverting forward slashes to back slashes wouldn't we?

That is my understanding of what @danieldresser meant, but don't want to speak for him. I think it would suffer from the same confusion of what is a file path and what is another path such as node hierarchy.

But regardless, our use of backslash as an escape character is incompatible with the storage of Windows native paths internally in Gaffer, isn't it? So that's a good argument for needing item 1.

I don't think that's 100% technically true but it speaks to why this all gets messy. You can use the double backslash in a string to result in a single backslash in the literal string just like always. For UNC paths that start with two back slashes, the string representation becomes "\\\\server\\path\\file.ext". (as a funny aside the GitHub comment system actually treats text inside quotes as though they are strings in a programming language so to write that path I needed to start it with 8 back slashes - case in point) I can definitely vouch for the idea that using back slashes in paths is annoying overall and a source of bugs, especially when getting used to Windows after working on Linux for a while.

Doing the automatic conversions we're discussing doesn't seem too hard for things like SceneReader and ImageReader, where it's really clear cut that they store filenames. But there are trickier cases that are actually really common, like using a string attribute to store texture filenames, and then querying that from a shader. In that case, we may or may not know they're filenames on input, and we're also not in control of how they'll be used in the renderer. What would we want to happen there? Should we identify the input string as a filename on the CustomAttributes node that creates it, so we convert to forward slashes for storage? But then convert to the backslash form when inserting the attribute into the scene itself?

This part is tricky. I'm starting to wonder if all of these questions add up to adding a new plug like FilePathPlug? It seems like that and maybe some tweaks to the FileSystemPath (or a new data type?) would address all of these issues. When querying them in a graph execution, the FilePathPlug would return the OS specific version, so anything throughout Gaffer, including renderers reading data from CustomAttributes, would only ever know about the OS specific form.

Not sure what the repercussions on backwards compatibility would be, but hopefully they would be minimal. Anyone using Gaffer now is Linux-only so they can continue to use strings forever if they like, right? It's the cross platform (or all Windows users probably) people who would need to adopt the new plug, and since they are going to be new to it all anyways hopefully not much of an additional learning curve.

And what do we do when the attribute is inside a cache file that's out of our control? Punt?

I say punt. My justification is anyone working cross-platform has probably already faced problems with caches and figured out some way to solve them. Deadline for example uses naive string substitution in a pre-task step and I've found that to be successful for the Nuke and Vray scene files I work with.

Though I'd need some convincing to be swayed, that argument could also be used to decline this whole discussion altogether and just pass paths straight through from the OS to Gaffer, ugly back slashes and all. In practice this may work for studios that have a clear separation of workstation to render farm - i.e. the render queue system could do all the path substitutions. I fear that in an interconnected program like Gaffer this would eventually lead to potentially show-stopping use of Gaffer, but I figured I'd throw it in the mix for discussion.

Another possibility for the sake of discussion is to treat anything with a file path-like form to be a string that needs conversion. This seems risky but if you can be sure object names or other things living in a path hierarchy would never have, say, a form of /path1/path2/file.ext and that file names always would (so no files without an extension) then maybe it would be an acceptable method that avoids major changes.

Just to throw one more spanner in the works : what about Unicode? @ericmehl, we don't really handle this properly at all right now, and have an open ticket about it. The plan is to use UTF8 internally on the C++ side, and unicode objects in Python. I believe some Windows APIs will require us to convert from UTF8 to some other form before use?

Firstly, my experience with Unicode come entirely from Python where it's fairly straightforward. I found a good page about using Unicode on Windows - long story short from my read:

1. Windows expects UTF16 strings as input to most or all API functions so a conversion would need to be done for UTF8 strings used in API calls
2. gcc defines wchar to be 32 bits and Windows defines them as 16 bit. Frown face. Not entirely sure how this effects strings that are only ever processed on a single architecture like most paths in memory would be, but maybe something to keep in mind.

Boost looks like it has good functions for converting from UTF8 to UTF16, so we could probably rely on that where needed?

This might be another reason to switch to file names being stored in a FilePathPlug or similar?

[johnhaddon]

I'm starting to wonder if all of these questions add up to adding a new plug like FilePathPlug? It seems like that and maybe some tweaks to the FileSystemPath (or a new data type?) would address all of these issues. When querying them in a graph execution, the FilePathPlug would return the OS specific version, so anything throughout Gaffer, including renderers reading data from CustomAttributes, would only ever know about the OS specific form.

Yep, this is beginning to look like the right approach to me.

Not sure what the repercussions on backwards compatibility would be, but hopefully they would be minimal. Anyone using Gaffer now is Linux-only so they can continue to use strings forever if they like, right?

Correct. And in the spots like SceneReader where we replace a StringPlug with a FilePlug, old scripts will continue to load because FilePlug.setValue() will accept the same strings we serialised for the StringPlug originally.

Another possibility for the sake of discussion is to treat anything with a file path-like form to be a string that needs conversion.

I appreciate the effort to provide a less intrusive alternative, but I think in practice this would lead to unwanted side effects and confusion. Like the Zen of Python says, "Explicit is better than implicit...In the face of ambiguity, refuse the temptation to guess.".

Windows expects UTF16 strings as input to most or all API functions so a conversion would need to be done for UTF8 strings used in API calls...this might be another reason to switch to file names being stored in a FilePathPlug or similar?

I'm hoping this is orthogonal to the FilePlug question - in terms of dataflow Gaffer will always pass UTF8 strings around via std::string. So calling FilePlug::getValue() in a compute will return you a UTF8 string with the slash substitutions applied, but we'll only do the conversion to UTF16 for Windows at the point of calling the Windows API. In most cases, that won't even be in Gaffer code - it'll be library code in Cortex or OIIO that actually opens the files.

Since it seems like we're closing in on an approach that'll work, let me point out the one major oddity I can see with it. Let's say I'm on Windows, and I'm using a CustomAttributes node to create a string attribute which specifies a texture filename. I'll add a FilePlug to my CustomAttributes node, and use the UI to choose a path. This will convert from Windows backslash form to Gaffer's forward slash form, and that's what I'll see in the file chooser, and the NodeEditor. Then the CustomAttributes node will be computed, so it'll pull on the FilePlug, receive the Windows-specific path via substitutions, and then I'll see the Windows backslashed path in the SceneInspector where I inspect the attributes. And if I write that scene to a cache, I'll be baking that into the cache. The only way I can see of dealing with this is to push the substitution further down the chain, so instead of creating StringData attributes, we'd create FilePathData attributes, and those would then need to be substituted at the very last minute by the renderer backends.

I'm fairly convinced by the need for the FilePlug now, but less sure about the FilePathData, because that pushes deeper and deeper into other parts of Gaffer/Cortex. Since we need FilePlug before we can even contemplate FilePathData, I'm tempted to suggest we should try FilePlug on its own first, and then consider FilePathData based on the experience we gain in the process...

Maybe it's time to spin off a specific issue for this?

[ericmehl]

Then the CustomAttributes node will be computed, so it'll pull on the FilePlug, receive the Windows-specific path via substitutions, and then I'll see the Windows backslashed path in the SceneInspector where I inspect the attributes.

That doesn't feel entirely wrong to me actually, though I think I get where you are coming from. Perhaps it's a point-of-view thing - in my mind the scene inspector is looking at a more raw form of data than the node editor presents. For example, for the related topic of environment variable substitutions, I'd expect to see ${studio_root}/dir/file.ext in the node editor and C:\studio_directory\dir\file.ext in the scene inspector. Does that sound in keeping with how IE works in day-to-day production, or would you want to see the ${studio_root}/dir/file.ext in the scene inspector? I haven't gotten Gaffer into my production workflow yet so I have no strong feelings on the subject.

And if I write that scene to a cache, I'll be baking that into the cache. The only way I can see of dealing with this is to push the substitution further down the chain, so instead of creating StringData attributes, we'd create FilePathData attributes, and those would then need to be substituted at the very last minute by the renderer backends.

I'm not entirely sure what you mean by a cache in this case, can you give an example? Something like an .ass file, or a cache native to Gaffer?

If it's a renderer-native cache, I think having OS native paths is the desired behavior so the renderer can load the files, right? And that also relates to why I think punting on external caches is fine because it is a familiar issue with studios working across platforms.

Since we need FilePlug before we can even contemplate FilePathData, I'm tempted to suggest we should try FilePlug on its own first, and then consider FilePathData based on the experience we gain in the process...

That sounds good to me. I'm happy to take a crack at it if you don't mind starting me off with a little guidance. Though I will have somewhat limited time to dedicate to it, probably on the order of an hour or so a day and some time on weekends.

FYI I'm merging my latest work on Gaffer Dependencies and Cortex to the PR branches now, so those will be up shortly for review.

Maybe it's time to spin off a specific issue for this?

Definitely!

[johnhaddon]

That doesn't feel entirely wrong to me actually, though I think I get where you are coming from. Perhaps it's a point-of-view thing - in my mind the scene inspector is looking at a more raw form of data than the node editor presents. For example, for the related topic of environment variable substitutions, I'd expect to see ${studio_root}/dir/file.ext in the node editor and C:\studio_directory\dir\file.ext in the scene inspector. Does that sound in keeping with how IE works in day-to-day production, or would you want to see the ${studio_root}/dir/file.ext in the scene inspector? I haven't gotten Gaffer into my production workflow yet so I have no strong feelings on the subject.

You're right - there's a definite distinction between what you see in the NodeEditor and what you see in the SceneInspector, and this is actually a distinction I'm always trying to drill into people. The NodeEditor represents the editable input to a process, and the SceneInspector presents the read-only output from a process. We definitely expect to see the expanded versions of things in the SceneInspector, but I was just concerned that baking in platform-specifics there might be a bad thing.

I'm not entirely sure what you mean by a cache in this case, can you give an example? Something like an .ass file, or a cache native to Gaffer?

I was thinking of baking out a scene into an Alembic or USD cache, but an Ass file would have the same issue. One scenario would be to bake down a scene to send to another facility to use in another package, but I guess in this case you've got to be managing the file paths somehow anyway. If it doesn't seem a big concern to you then let's not trouble ourselves with it just yet - let's just start with FilePlug and see where we end up...

I'm happy to take a crack at it if you don't mind starting me off with a little guidance.

Great! Should we maybe start with some tests and fixes for FileSystemPath, demonstrating the conversion from Windows paths to the internal form? Or do we need the FilePlug first, so that when we update FileSystemPath, we've already got the reverse conversion in place?

[ericmehl]

I guess I'll start with tests and fixes on FileSystemPath since that seems more to the core and builds on existing code. Then I'll move to getting tests of the SceneReader and ImageReader working on Windows to verify ongoing work on those nodes, then add the FilePlug. Does that sound right?

Also moving this discussion to it's own thread so this one can return to the Appleseed discussion it was originally opened for.

[johnhaddon]

Closing, since we have removed Appleseed support from all platforms for Gaffer 1.3.