getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Add Blender 2.80 support (this will resolve #361) #466

Open jasperges opened 4 years ago

jasperges commented 4 years ago

So, this is a bigger one. :) Adding Blender 2.80+ as host to Avalon.

Some notes up front:

That being said, let's have a discussion how we can work towards adding this to Avalon. Do you see issues, is there missing functionality, did you encounter bugs, etc.?

Looking forward to your comments. :)

jasperges commented 4 years ago

Thank you hound... :)

jasperges commented 4 years ago

I think I've fixed the main issues for now. The Travis build failing is because of Python 3 specific syntax in the Blender part (as I already mentioned in my first comment).

mottosso commented 4 years ago

The Travis build failing is because of Python 3 specific syntax in the Blender part

Could you have a look to see whether you can fence Python 3-specific tests behind a if PYTHON_MAJOR == 3 block (using whichever naming convention we've got for that currently, if any)? Otherwise we could have a look at a dedicated CI pass for Python 3, but that would be a little unfortunate.

for example the type annotations. How do you feel about this?

Good question. I would like us to use them, but it will be a good few years before anyone in the Maya-camp can actually join in, as it would mean stripping support for Python 2 altogether. They would make reading and understanding more challenging for anyone until that time, but I also don't think it's a good idea to hold Blender back due to a slowly moving remainder of the VFX industry.

We'll have to pay attention to what is exclusive to Blender, such as utility functions that may or may not be useful elsewhere, but it shouldn't be too hard considering we've got the avalon.blender namespace. So my vote is "Go for it", but everyone should weigh in on this, as it would affect their ability to understand, contribute and debug this aspect of the codebase as well.

For completeness, not sure this works from a comment, but to trigger the GitHub automation gods, this PR fixes #361 (which should automatically close that upon merging this PR).

Finally, merging this may be tricky, for the same reason https://github.com/getavalon/core/pull/438 is challenging; testing it is hard. Blender + Avalon is still a niche, so this will be likely be your baby, at least initially. I'm happy delegating responsibility for this section of the codebase - ensuring code consistency and that it works - to you, if you'd be happy with that.

Finally again, can you post screenshots? :) I think we're all eager to see it in action, Blender user or no.

And finally, great work!! Wonder if this would make us eligable for an announcement on one of the many Blender forums? Following the latest convention, I suspect people are in an open frame of mind with regards to switching things around and trying new things.

jasperges commented 4 years ago

For completeness, not sure this works from a comment, but to trigger the GitHub automation gods, this PR fixes #361 (which should automatically close that upon merging this PR). That's why I have 'this will resolve #361' in the title. I believe that should work... :)

Screenshots etc.: yes, I should definitely show them somewhere and probably make some documentation... Where would you like for screenshots/documentation to go?

I will get back later to your other points.

BigRoy commented 4 years ago

Because Blender 2.80 uses Python 3.7 there is some specific Python 3 stuff in there, for example the type annotations. How do you feel about this?

Agreed. Go for it, as long as it's only inside the code areas that are supposed to only have be Python 3+ where it has support for it, e.g. the avalon.blender space.

Finally again, can you post screenshots? :) I

Moving footage of it working, like publishing and loading/update/removing content would be amazing!

mkolar commented 4 years ago

We actually recently started using blender for certain tasks, so we'll be able to test it out fairly quickly I think. I'll try to find time today to pull it and give it a spin. Looking forward

jasperges commented 4 years ago

Could you have a look to see whether you can fence Python 3-specific tests behind a if PYTHON_MAJOR == 3 block (using whichever naming convention we've got for that currently, if any)? Otherwise we could have a look at a dedicated CI pass for Python 3, but that would be a little unfortunate.

Yes, I can definitely have a look at that. Can you point me in the right direction where I can find the relevant code/tests? I had a look at the test things, but couldn't quite figure out what is exactly run on Travis. Also looking at this, maybe it would make sense to make some tests for Blender, like there is for Maya. Because it's open source we could easily create a docker with Blender to run some tests. How do you feel about that?

Finally, merging this may be tricky, for the same reason #438 is challenging; testing it is hard. Blender + Avalon is still a niche, so this will be likely be your baby, at least initially. I'm happy delegating responsibility for this section of the codebase - ensuring code consistency and that it works - to you, if you'd be happy with that.

I completely understand it's tricky to merge this. If I could chop it into smaller pieces, I would, but I don't see any way to do that. I would be happy to take the responsibility for the blender part. Personally I think this can be considered alpha or maybe beta stage. It will mature over time as we use it more and hopefully other people join the party.

Finally again, can you post screenshots? :) I think we're all eager to see it in action, Blender user or no.

In due time, have patience... :stuck_out_tongue: What would be a good place to put some documentation for Blender part? Would it be okay, to put an extra README.md in avalon/blender? Or would you prefer it to be in the docs repo?

And finally, great work!! Wonder if this would make us eligable for an announcement on one of the many Blender forums? Following the latest convention, I suspect people are in an open frame of mind with regards to switching things around and trying new things.

Thanks! Before posting it I would prefer let it mature a bit. Also the configuration for the Blender part needs more, so there is actually something to show.

jasperges commented 4 years ago

Because Blender 2.80 uses Python 3.7 there is some specific Python 3 stuff in there, for example the type annotations. How do you feel about this?

Agreed. Go for it, as long as it's only inside the code areas that are supposed to only have be Python 3+ where it has support for it, e.g. the avalon.blender space.

Cool. And yes, it will only be in avalon.blender and setup/blender.

Moving footage of it working, like publishing and loading/update/removing content would be amazing!

Yes, yes... I will dig up some nice looking assets, possibly from one of the Bender movies so I have more to show then just some cubes and monkeys.

jasperges commented 4 years ago

We actually recently started using blender for certain tasks, so we'll be able to test it out fairly quickly I think. I'll try to find time today to pull it and give it a spin. Looking forward

Cool! Looking forward to hear how it works for you.

mkolar commented 4 years ago

So I have a few comments / questions.

I've managed to make it work in pype config (just copying the blender parts). I'm running it on windows and run into some issues. Creator and Workfiles are not loading. I didn't dig into code details before checking that on linux all the key parts are working for you.

What I'm currently struggling with is Creator that crashes with this

Traceback (most recent call last):
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\blender\ops.py", line 248, in execute
    return super().execute(context)
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\blender\ops.py", line 212, in execute
    self._window.show(*args, **kwargs)
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\tools\creator\app.py", line 630, in show
    window = Window(parent)
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\tools\creator\app.py", line 142, in __init__
    name = SubsetNameLineEdit()
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\tools\creator\app.py", line 73, in __init__
    anim.setPropertyName("status_color")
TypeError: setPropertyName(self, Union[QByteArray, bytes, bytearray]): argument 1 has unexpected type 'str'

location: <unknown location>:-1

And workio that can't see registered host that I'm able to trace back to api.install(blender) that throws this

Exception ignored in: <function ImagePreviewCollection.__del__ at 0x000001961F2FFF28>
Traceback (most recent call last):
  File "C:\Program Files\Blender Foundation\Blender\2.80\scripts\modules\bpy\utils\previews.py", line 79, in __del__
    f"{self!r}: left open, remove with "
ResourceWarning: <ImagePreviewCollection id=0x1961f77fe60[1], <super: <class 'ImagePreviewCollection'>, <ImagePreviewCollection object>>>: left open, remove with 'bpy.utils.previews.remove()'
Traceback (most recent call last):
  File "<blender_console>", line 1, in <module>
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\pipeline.py", line 81, in install
    host.install(config)
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\blender\pipeline.py", line 118, in install
    ops.register()
  File "C:\code\pype-setup-2.0\repos\avalon-core\avalon\blender\ops.py", line 395, in register
    bpy.utils.register_class(cls)
ValueError: register_class(...): already registered as a subclass

I'm assuming this is not happening to you @jasperges

BigRoy commented 4 years ago

What I'm currently struggling with is Creator that crashes

Should be fixed with https://github.com/getavalon/core/pull/469

The second one sounds like you are running install multiple times on the same code. Not sure...

jasperges commented 4 years ago

@mkolar Nope, that doesn't happen to me.

As @BigRoy mentions, the first error should be fixed if you update Avalon.

The second issue also feels to me like Roy mentions. This kind of thing mostly happens to me if the register function in Blender fails and you try to do it a second time (for example reloading a broken add-on). Also I'm not sure if this part:

Exception ignored in: <function ImagePreviewCollection.__del__ at 0x000001961F2FFF28>
Traceback (most recent call last):
  File "C:\Program Files\Blender Foundation\Blender\2.80\scripts\modules\bpy\utils\previews.py", line 79, in __del__
    f"{self!r}: left open, remove with "
ResourceWarning: <ImagePreviewCollection id=0x1961f77fe60[1], <super: <class 'ImagePreviewCollection'>, <ImagePreviewCollection object>>>: left open, remove with 'bpy.utils.previews.remove()'

is related to the Avalon Blender integration. It could be because I 'register' the Pyblish icon as a custom icon. If it is, something is definitely wrong and an earlier host.install probably failed causing these errors running the install a second time.

Today I have some Windows testing planned. So let's see how that will work out.

BigRoy commented 4 years ago

@mkolar are you able to give this another go shortly? :)

jasperges commented 4 years ago

@BigRoy @mkolar On Windows there are issues with getting the proper context from Blender. After a discussion with @mottosso I'm now implementing the communication between Qt and Blender with a queue.Queue. That seems to work. Forgot to update this PR and issue #361. Sorry, about that. I hope I will have a (somewhat) working version on Windows tomorrow.

mkolar commented 4 years ago

We've worked with this as we have blender requirement now from a client. It's running fine on windows now. What we found though is that the plugin registration has a tendency to crash and because Blender is not running python in another thread, it actually takes the whole application with it. @jasperges didi you have any similar issues?

BigRoy commented 4 years ago

@mkolar does this sys.excepthook trick help?

Just wondering. :)

jasperges commented 4 years ago

@mkolar Yes, I did have similar issues. My 'solution' was to launch Blender from a shell so I could inspect the output after it crashed. As @BigRoy mentions the sys.excepthook trick shoud prevent this from happening. Can you check one thing for me? If you (for example in the Workfiles app, or easier use AVALON_DEBUG=1 and use the Test app) query bpy.context.object (active object) and of course have an object selected, does it return the object or is it empty/gives an error? Nevermind the last thing. @iLLiCiTiT already checked for me.

jasperges commented 4 years ago

I added avalon.blender.bpy. This is a simple wrapper around Blender's bpy. The reason I added this, is to work around the problem when querying bpy.context from a Qt app on Windows. I would still like to resolve the Qt vs Blender problems, but for now this is a quick work around to get things working on both Linux and Windows (unfortunately is totally doesn't work macOS). For now only bpy.context.active_object, bpy.context.object and bpy.context.selected_objects are added when they do not exist. But it's quite simple to add more if needed. In all Avalon related code you should use from avalon.blender import bpy instead of import bpy. If at some point this is not needed anymore, avalon.blender.bpy can just serve bpy unaltered. Or you can do a quick search and replace of course.

More information about this problem will be documented here.

iLLiCiTiT commented 4 years ago

We examined a little bit and actually don't understand how it is working on linux? Because each "window" has it's Context. There is Global Context where scene, window, window_manager, etc. are but selected_objects attribute is only in Screen Context which is not active in the moment of processing QApplication. So to get selection you should do, what you do in bpy.py.

[obj for obj in bpy.context.scene.objects if obj.select_get()]

Same for data, you can't acces workspaces with bpy.data.workspaces but bpy.context.blend_data.workspaces.

But I would not use that with "overriding" bpy but implement lib methods instead, to be more readable and easy understandable to how it works. What do you think?

iLLiCiTiT commented 4 years ago

We've used your code, changed context data access and it works! You can check our branch. To make it work with plugins, you'll have to change bpy.data to bpy.context.blend_data and if you want to get selection, you have to use avalon.blender.lib.get_selection() if you use our branch. It is just a method returning [obj for obj in bpy.context.scene.objects if obj.select_get()]. We're happy and able to publish and load in blender on windows :)

jasperges commented 4 years ago

@iLLiCiTiT I'm guessing because of how the windows are managed on Linux you have the Screen Context even in from within the Qt window (maybe in Windows it's a real separate window and on Linux it's a child window?). But I have no idea.

Probably because of my history with Blender and being so used to using bpy.context.object and bpy.context.selected_objects etc. I wanted to have those available within Avalon. That's why I made the 'wrapper'. But now that I had some more time to think about it, I prefer your solution. In the documentation (that I am planning to write for the Blender part) it should be made very clear that you can only expect to have access to the Global Context. And as you did provide some utility functions to 'ease the pain'.

By the way: what issues did you have with using bpy.data. In my quick tests I didn't have any problems with it. So I'm curious why you're using bpy.context.blend_data.

iLLiCiTiT commented 4 years ago

By the way: what issues did you have with using bpy.data. In my quick tests I didn't have any problems with it. So I'm curious why you're using bpy.context.blend_data.

Hi, I'm in the middle of preparing New Year's goodies, so briefly: My goal was to be able to use creators and publish and when found out that most of used attributes are not in global context, I changed them to bpy's global context attributes which worked and didn't re-check if bpy.data works because by blender's documentation it is not part of global context.

My main issue was that blender is changing python version in 2.80 and 2.81 so it is not possible to use one version of PyQt/PySide for them.

I can test if bpy.data works on 2.1.2020. But if it works for you, I guess it will work for me ;)

jasperges commented 4 years ago

@iLLiCiTiT Thanks for the quick answer. I think bpy.data should just work. It basically is the data of the current blend file (which should always be available). It might be that bpy.context.blend_data and bpy.data point to the same object, but I haven't checked it. Unless there are problems I will stick to bpy.data for now. Take your time for testing bpy.data, there is absolutely no rush.

By the way: I changed the way the Qt apps are kept 'alive'. Now I use bpy.app.timers.register with persistent=True, so apps should continue working, even if you open a new file. The added benefit is that it now works consistently on Linux, Windows and macOS. On Linux you also 'only' get the Global Context with bpy.context.

jasperges commented 4 years ago

In the function I register with bpy.app.timers.register I do a simple check to see if an QApplication needs to processEvents. I do this by checking if the QApplication has any top level windows (QApplication.topLevelWindows()) and if any of these top level windows are visible (window.isVisible()) (See here.) Do you guys think this solution is fine or are there better ways?

jasperges commented 4 years ago

Added some comments that would be necessary changes when PR #501 is merged.

Thanks for the heads up. Will update once #501 is merged.

iLLiCiTiT commented 4 years ago

Take your time for testing bpy.data, there is absolutely no rush.

I've checked, and yes it works with bpy.data the same way so bpy.context.blend_data is not necessary :)

Do you guys think this solution is fine or are there better ways?

I don't see any other possible solution at this moment and I would doubt there is a better, this is only 100% sure way. Question: Is possible to start blender timer on show and stop the timer if any window is visible? (didn't investigate your solution so I don't know if you already do that)

jasperges commented 4 years ago

I've checked, and yes it works with bpy.data the same way so bpy.context.blend_data is not necessary :)

Nice. Thanks for checking.

Question: Is possible to start blender timer on show and stop the timer if any window is visible? (didn't investigate your solution so I don't know if you already do that)

At the moment a timer is started when you show an application. This is implemented in the LaunchQtApp operator. If no Qt windows are visible the registered (timed) functioon returns None so it is unregistered. The only problem is that for every app you show, a new timer is registered (unless Blender does a good check in the background), so that is not optimal. Will fix that in a future commit.

iLLiCiTiT commented 4 years ago

And what about to do check before registering timer? Same as QApplication.topLevelWindows() -> window.isVisible()

jasperges commented 4 years ago

You mean to avoid registering multiple timers? If so, yes, I will implement a check before registering.

jasperges commented 4 years ago

Could you have a look to see whether you can fence Python 3-specific tests behind a if PYTHON_MAJOR == 3 block (using whichever naming convention we've got for that currently, if any)? Otherwise we could have a look at a dedicated CI pass for Python 3, but that would be a little unfortunate.

Yes, I can definitely have a look at that. Can you point me in the right direction where I can find the relevant code/tests? I had a look at the test things, but couldn't quite figure out what is exactly run on Travis. Also looking at this, maybe it would make sense to make some tests for Blender, like there is for Maya. Because it's open source we could easily create a docker with Blender to run some tests. How do you feel about that?

@mottosso Can you shed some light on this? ^^ I would like to fix the tests on Travis, but couldn't figure out how it works.

jasperges commented 4 years ago

If possible I would like to merge this integration in the near future. The things I would like to do before a merge:

Do you have any thoughts on this? Do you see any problems or things that need to be fixed or changed?

iLLiCiTiT commented 4 years ago

I would add sys.excepthook = lambda *exc_info: traceback.print_exception(*exc_info) into blender's pipeline install method to not crash blender because of unhandled exceptions in avalon.

BigRoy commented 4 years ago

Good list @jasperges - hit me up if you have a simple test config, etc. and you want me to run through it once too.

For the README.md maybe these Avalon Integration notes I wrote out could be helpful. I noticed it was non-trivial to know which environment variables to set per integration so set this up as a reference.

mottosso commented 4 years ago

I would add sys.excepthook = lambda

Don't!

Consider what happens if another vendor did that at the same time, or if Blender themselves at one point installed one for their own internal purposes. There can only be one of those; Avalon should be able to handle exceptions internally, to carry its own weight.

If not, then we need to give the user an option to install it, explaining the risks (such as random breakage of anything else that depend on it, including Blender).

I had a look at the test things, but couldn't quite figure out what is exactly run on Travis.

I think we solved this quite well in the Qt.py project, see here for example.

    if PYTHON == 2:
        def test_sip_api_already_set():
            """Raise ImportError with sip was set to 1 with no hint, default"""
            __import__("PyQt4.QtCore")  # Bypass linter warning
            import sip

If would simply exclude that block of code in anything that isn't Python 2. The more complicated option would be to have separate modules, and run the Python 2 interpreter for one, and Python 3 for another. We would need to modify the CI file for that which can get complicated fast.

In general, still strive to write code that run on both Python's so that you won't need such an if-block. It'll still be necessary to support both in parallel for at least another few years.

mottosso commented 4 years ago

Oh my, tests are running in Python 2.6? :O I don't see how, it should be using mayapy (which is 2.7).

Unsure about that one.

jasperges commented 4 years ago

@mottosso The fix was actually very simple. Nose was failing during the discovery of tests, no code was actually tested. Maya and Houdini were already excluded, now I added Blender (here and here) and the problem is gone.

Oh my, tests are running in Python 2.6? :O I don't see how, it should be using mayapy (which is 2.7).

It seems that either mayapy -m pip install nose installs it globally (for the system Python 2.6) or Nose is already installed system wide and that one is picked up by mayapy. Would have to inspect the Docker images to fully understand what's happening. Weird...

jasperges commented 4 years ago

I would add sys.excepthook = lambda

Don't!

Consider what happens if another vendor did that at the same time, or if Blender themselves at one point installed one for their own internal purposes. There can only be one of those; Avalon should be able to handle exceptions internally, to carry its own weight.

If not, then we need to give the user an option to install it, explaining the risks (such as random breakage of anything else that depend on it, including Blender).

Given these considerations I see 2 options:

  1. 'Hide' it behind an environment variable or something similar.
  2. Add an explanation and instructions how to add the sys.excepthook to README.md for the Blender integration (which I'm writing now). You can easily add it in core/setup/blender/startup/avalon_setup.py or add your own startup script for Blender that does it.

Personally I would prefer the second option.

jasperges commented 4 years ago

Good list @jasperges - hit me up if you have a simple test config, etc. and you want me to run through it once too.

Thanks, will do.

For the README.md maybe these Avalon Integration notes I wrote out could be helpful. I noticed it was non-trivial to know which environment variables to set per integration so set this up as a reference.

That is exactly what I was doing. :smile:

Nice resource by the way. At some point I quickly wanted to test Avalon with Houdini (Apprentice) but didn't have a clue how to set it up.

iLLiCiTiT commented 4 years ago

@jasperges any updates about blender?

Few questions: 1.) Is same issue happening (had happened) to you? 2.) Are you solving that or have you solved that? 3.) If not what do you (and others) think about that different approach? (I can share branch if necessary)

jasperges commented 4 years ago

@iLLiCiTiT A quick answer for now. Will check more thoroughly on Monday.

  1. Is this on Windows? This could potentially be something that happens only Windows. I mainly tested this on Linux and I don't recall having this problem. At least not frequently. In the future maybe we should use bqt. Then probably things like this can be fixed over there and everyone benefits. But that would mean getting support for Linux and macOS first.
  2. Like with 1, I can't recall having seen this problem. Will carefully check again, especially on Windows.
  3. If possible I would like to avoid by-passing the show function of each tool module. But we have to see if that is possible.
jasperges commented 4 years ago

@iLLiCiTiT I double checked and and do not have the problem you describe in 1 and 2. In Blender I do not keep track of the module.window directly. There is one odd thing though: because the Loader handles the window a bit differently I have put in some extra code there. This seems to prevent the error you are having. I also have this PR which should resolve that issue.

Could it be that you call the tools differently then I do or have a different approach of getting the QApplication instance? It also seems related to this: https://github.com/getavalon/core/issues/514

jasperges commented 4 years ago

@iLLiCiTiT I'm now hitting the same ACCESS VIOLATION error as you did. I have the error when trying to open a file with the new workfiles app. It seems it somehow is caused when running Pyblish. So far this is my 'recipe for disaster': open Blender > open workfiles app, opening file and closing workfiles app > open Pyblish and close it > open new file > open workfiles app, open file >> crash.

I don't have time to further investigate the issue right now, but I will get back about this. I really don't know if it is in any way related, but Pyblish uses a 'modal operator'. I will re-work it so it uses the new bpy.app.timer and see what happens then. If that doesn't change anything, I will check your solution.

To be honest I'm stumbling in the dark here, but we'll see...

jasperges commented 4 years ago

@iLLiCiTiT Finally had some time today to dive deeper into the issue. Apart from the Access Violation error in Windows I kept getting 'Segmentation Faults' when opening files with the Workfiles tool in Linux. After lots of digging around and trying stuff out, I figured out what the problem is. If you open a .blend file from within a 'timed' function, Blender crashes. So, this will crash Blender:

import bpy

count = 0

def timed_function():
    global count
    count += 1
    print(f"Crashing in {count}...")
    if count > 2:
        bpy.ops.wm.open_mainfile(filepath=<blendfilepath>)
    return 1  # run every second

bpy.app.timers.register(timed_function, persistent=True)

On Windows it might take a second try. But after it has crashed it will crash consistently afterwards unless you log out in back in again (in my tests).

Now to find a solution for this...


EDIT: this might be related to these:

EDIT 2: It seems that at the moment you can't run operators from within timed functions. For some you might be able to override the context, but I haven't been able to do this for bpy.ops.wm.open_mainfile. As a (temporary) solution I switched back to using a modal operator. To prevent windows getting stuck when the modal operator stops (for example when opening a new file) I simply close all Qt windows that are managed by the current QApplication.instance(). It's not great, but at least it makes it workable again...

You can check this out in this branch.