Details

Reviewers

Michael Parkin-White (MichaelPW)
Campbell Barton (campbellbarton)

Maniphest Tasks

T103863: Metal Backend: Add fallback for Add-ons that use BGL or Incorrect shader-code with GPU module.

Commits

rBcac6b6f38881: BGL_Wrap: disable calls on non-opengl backends.

Summary

Goal of this patch is to stop the invocation of OpenGL calls via the bgl module
on a none OpenGL GPU backend, report this as a python deprecation warning
and report this to the user.

Deprecation warning to developers

>>> import bgl
>>> bgl.glUseProgram(0)
<blender_console>:1: DeprecationWarning: 'bgl.glUseProgram' is deprecated and will be removed in Blender 3.7. Report or update your script to use 'gpu' module.

Deprecation message to users

The message to the user is shown as part of the Info Space and as a message box.

During implementation we tried several ideas:

Use python warning as errors: This isn't fine grained enough and can show incorrect information to the user.
Throw deprecation as error and use sys.excepthook to report the user message. This required a custom exception class to identify the bgl deprecation and a CPython handler function to be set during python initialization. Although this is the most flexible there was a disconnect between the exception class, exception function and the excepthook registration.
A variant how we handle autoexec failures. A flag is stored in Global and when set the user message is reported. Not that flexible, but code is more connected to the boolean stored in the Global struct.

Although using Global struct isn't nice I chose this solution due to its traceability. It is clear to developers
reading the code how the mechanism works by using search all functionality of your IDE.

Diff Detail

Repository: rB Blender

Event Timeline

Jeroen Bakker (jbakker) created this revision.Fri, Jan 13, 4:38 PM

Harbormaster completed remote builds in B25362: Diff 59419.Fri, Jan 13, 4:38 PM

Jeroen Bakker (jbakker) added a task: T103863: Metal Backend: Add fallback for Add-ons that use BGL or Incorrect shader-code with GPU module..Fri, Jan 13, 4:39 PM

Show in log when bgl is imported without an OpenGL back-end being active.
Report deprecation warning.

Harbormaster completed remote builds in B25378: Diff 59482.Mon, Jan 16, 2:42 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Mon, Jan 16, 2:43 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Fix system error when running with metal.

Harbormaster completed remote builds in B25379: Diff 59483.Mon, Jan 16, 2:49 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Mon, Jan 16, 2:50 PM

Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).Mon, Jan 16, 3:06 PM

Germano Cavalcante (mano-wii) added inline comments.

source/blender/python/generic/bgl.c
1122	The deprecated warning is often overlooked. The error message cannot be ignored. If we wanted to show something more informative, (instead of `"returned NULL without setting an exception"`) we could use: `PyErr_Format` or `PyErr_SetString`

Small tweaks to the error message.
Use error when running with metal.

Harbormaster completed remote builds in B25381: Diff 59485.Mon, Jan 16, 3:38 PM

Jeroen Bakker (jbakker) added inline comments.Mon, Jan 16, 3:41 PM

source/blender/python/generic/bgl.c
1122	This would still not show to the user on a mac. I believe the best option is to load sys.excepthook with a custom callback that reports back to the user. Still investigating how to do this via CPython

WIP: exception handler for deprecation warnings.

Harbormaster completed remote builds in B25382: Diff 59486.Mon, Jan 16, 4:03 PM

Michael Parkin-White (MichaelPW) added a subscriber: Michael Parkin-White (MichaelPW).Mon, Jan 16, 6:05 PM

Use similar mechanism as autoexec fail to show message to user.

Harbormaster completed remote builds in B25394: Diff 59510.Tue, Jan 17, 8:39 AM

@Campbell Barton (campbellbarton) Although this patch is not yet finished. I would like an initial review on the approach as we discussed a different solution.

Show popup panel with message and mitigation.

Harbormaster completed remote builds in B25395: Diff 59514.Tue, Jan 17, 10:46 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Tue, Jan 17, 10:46 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Tue, Jan 17, 10:50 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Removed commented out code.

Harbormaster completed remote builds in B25396: Diff 59515.Tue, Jan 17, 10:53 AM

Jeroen Bakker (jbakker) added a project: BF Blender (3.5).Tue, Jan 17, 10:54 AM

Eugen (mycoconut) awarded a token.Tue, Jan 17, 12:20 PM

Eugen (mycoconut) added a subscriber: Eugen (mycoconut).

Jeroen Bakker (jbakker) added inline comments.Tue, Jan 17, 12:34 PM

source/blender/windowmanager/intern/wm_window.c
1648	Add newline

Campbell Barton (campbellbarton) added inline comments.Tue, Jan 17, 1:10 PM

source/blender/python/generic/bgl.c
1124	Is it possible to continue with the functions logic (even if the BGL call does nothing), as any function that is expected to return a non `None` value will error (although admittedly there aren't too many of these).
source/blender/windowmanager/intern/wm_window.c
1659–1668	It would be good to include the source of the error, otherwise it's up to the user to find which script caused the problem. The `Global` could store the location of the error and report it here, suggest to use `PyC_FileAndNum` in `report_deprecated_call_to_user` to include the file and line the BGL call was made.

Jeroen Bakker (jbakker) added inline comments.Tue, Jan 17, 1:20 PM

source/blender/python/generic/bgl.c
1124	The biggest issue is that on metal the opengl library isn't loaded, that will crash when continuing the logic. Adding logic to use the correct return type might be doable, For example -1 for integer types. Will have a look how to do this
source/blender/windowmanager/intern/wm_window.c
1659–1668	Note that this is already reported in the Python warning, just didn't want to clutter this information to the end-user. But no hard objections adding some hints here as well.

Eugen (mycoconut) removed a subscriber: Eugen (mycoconut).Tue, Jan 17, 1:36 PM

Code-style: Added empty line
Show hint of error to end user.

Harbormaster completed remote builds in B25401: Diff 59522.Tue, Jan 17, 2:29 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Tue, Jan 17, 2:30 PM

Jeroen Bakker (jbakker) marked an inline comment as done.

Campbell Barton (campbellbarton) accepted this revision.Wed, Jan 18, 7:35 AM

Campbell Barton (campbellbarton) added inline comments.

source/blender/python/generic/bgl.c
52	Use `(void)`, to prevent `-Wstrict-prototypes` warning.
source/blender/windowmanager/intern/wm_window.c
1659–1668	While I see your point - if an inexperienced user runs into this message they wont know who to report the issue to if there are no hints as to the cause of the warning. An alternative could be to tell the user to run Blender from a terminal, but that feels like asking them to perform extra/unnecessary steps if the file/line is sufficient.
1661	picky perhaps this should say "Python script uses OpenGL for drawing" ... as it's possible the script is not an add-on.

This revision is now accepted and ready to land.Wed, Jan 18, 7:35 AM

This appears to work as expected through testing, thanks!
Blender no longer appears to crash when enabling add-ons via the Metal backend, but highlights useful error messages.

Michael Parkin-White (MichaelPW) accepted this revision.Wed, Jan 18, 2:31 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Thu, Jan 19, 8:13 AM

Fix strict prototypes.
Improved title text.

Harbormaster completed remote builds in B25447: Diff 59614.Thu, Jan 19, 8:18 AM

Closed by commit rBcac6b6f38881: BGL_Wrap: disable calls on non-opengl backends. (authored by Jeroen Bakker (jbakker)). · Explain WhyThu, Jan 19, 8:19 AM

This revision was automatically updated to reflect the committed changes.

Jeroen Bakker (jbakker) added a commit: rBcac6b6f38881: BGL_Wrap: disable calls on non-opengl backends..

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Thu, Jan 19, 8:22 AM

BGL_Wrap: disable calls on non-opengl backends.
ClosedPublic
Actions

Details

Deprecation warning to developers

Deprecation message to users

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 59615

source/blender/blenkernel/BKE_global.h

source/blender/python/generic/CMakeLists.txt

source/blender/python/generic/bgl.c

source/blender/windowmanager/intern/wm_event_system.cc

source/blender/windowmanager/intern/wm_window.c

source/blender/windowmanager/intern/wm_window_private.h

BGL_Wrap: disable calls on non-opengl backends.ClosedPublicActions

Details

Deprecation warning to developers

Deprecation message to users

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 59615

source/blender/blenkernel/BKE_global.h

source/blender/python/generic/CMakeLists.txt

source/blender/python/generic/bgl.c

source/blender/windowmanager/intern/wm_event_system.cc

source/blender/windowmanager/intern/wm_window.c

source/blender/windowmanager/intern/wm_window_private.h

BGL_Wrap: disable calls on non-opengl backends.
ClosedPublic
Actions

Revision Contents
Changeset List