core icon indicating copy to clipboard operation
core copied to clipboard

Add Blender 2.80 support (this will resolve #361)

Open jasperges opened this issue 6 years ago • 43 comments

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

Some notes up front:

  • It is tested, but not thoroughly yet and only on Linux. I hope to have some artists who will start using it soon (mainly on Windows).
  • 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?
  • To play around with it, a config that supports Blender is needed. You can try this one. It's basically the Colorbleed config with a few Blender things. I expect to add extra things to this configuration in the next weeks, which will also enable me to better test the integration itself.

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 avatar Oct 30 '19 14:10 jasperges

Thank you hound... :)

jasperges avatar Oct 30 '19 14:10 jasperges

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).

jasperges avatar Oct 30 '19 15:10 jasperges

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.

mottosso avatar Oct 30 '19 17:10 mottosso

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.

jasperges avatar Oct 30 '19 17:10 jasperges

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!

BigRoy avatar Oct 30 '19 17:10 BigRoy

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

mkolar avatar Oct 31 '19 08:10 mkolar

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 avatar Nov 01 '19 11:11 jasperges

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 avatar Nov 01 '19 11:11 jasperges

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.

jasperges avatar Nov 01 '19 11:11 jasperges

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

mkolar avatar Nov 19 '19 21:11 mkolar

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...

BigRoy avatar Nov 20 '19 06:11 BigRoy

@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.

jasperges avatar Nov 20 '19 07:11 jasperges

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

BigRoy avatar Dec 12 '19 08:12 BigRoy

@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.

jasperges avatar Dec 12 '19 11:12 jasperges

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?

mkolar avatar Dec 13 '19 07:12 mkolar

@mkolar does this sys.excepthook trick help?

Just wondering. :)

BigRoy avatar Dec 13 '19 08:12 BigRoy

@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 avatar Dec 13 '19 08:12 jasperges

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.

jasperges avatar Dec 18 '19 15:12 jasperges

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 avatar Dec 19 '19 11:12 iLLiCiTiT

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 :)

iLLiCiTiT avatar Dec 19 '19 16:12 iLLiCiTiT

@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.

jasperges avatar Dec 30 '19 14:12 jasperges

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 ;)

iLLiCiTiT avatar Dec 30 '19 19:12 iLLiCiTiT

@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 avatar Dec 31 '19 13:12 jasperges

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 avatar Dec 31 '19 13:12 jasperges

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

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

jasperges avatar Jan 02 '20 11:01 jasperges

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)

iLLiCiTiT avatar Jan 02 '20 13:01 iLLiCiTiT

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.

jasperges avatar Jan 03 '20 10:01 jasperges

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

iLLiCiTiT avatar Jan 03 '20 10:01 iLLiCiTiT

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

jasperges avatar Jan 03 '20 11:01 jasperges

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 avatar Jan 08 '20 15:01 jasperges