Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][FIX] Fix rendering when height/width is zero #2793

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

robyf70
Copy link
Contributor

@robyf70 robyf70 commented Apr 9, 2024

The bokeh v3.1.1 has a issue when the canvas has width/height at zero. This happen when moving from a view form to tree view and the Bokeh widget is going to be destroyed. Javascript console report issue below. The fix has been introduced in v3.2.x of the library.

InvalidStateError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The image argument is a canvas element with a width or height of 0.
    at g.blit_webgl (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:464:2802) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:464)
    at _._paint_levels (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582:15891) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582)
    at _._actual_paint (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582:15197) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582)
    at _.paint (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582:14153) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582)
    at _._after_layout (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582:13990) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:582)
    at _.after_layout (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547:6160) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547)
    at _.compute_layout (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547:5540) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547)
    at _._after_resize (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547:732) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:547)
    at _.after_resize (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:430:2547) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:430)
    at ResizeObserver.<anonymous> (web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:430:2043) (http://localhost:8069/web_widget_bokeh_chart/static/src/lib/bokeh/bokeh-3.1.1.min.js:430)

@OCA-git-bot
Copy link
Contributor

Hi @ChrisOForgeFlow, @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@robyf70 robyf70 force-pushed the 16.0-web_widget_bokeh_chart_upgrade_to_fix_rendering_height_0 branch from e35ecd9 to 21382db Compare April 9, 2024 10:31
@LoisRForgeFlow
Copy link
Contributor

Hi @robyf70

We don't update the library version once merged in an odoo stable version, as it affect existing usages of the widget.

The migration to v17 or v18 will be the oportunity to do this update.

Sorrry for the inconvenience.

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 9, 2024

Hi @robyf70

We don't update the library version once merged in an odoo stable version, as it affect existing usages of the widget.

The migration to v17 or v18 will be the oportunity to do this update.

Sorrry for the inconvenience.

Hi @LoisRForgeFlow,

I agree 100% with you, however what about this case if the library is breaking the navigation and crash?

@LoisRForgeFlow
Copy link
Contributor

@robyf70 Do you have a use case that you can share? Maybe you can point to the bokeh chart computation and/or share a video of how it crashes.

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 9, 2024

@robyf70 Do you have a use case that you can share? Maybe you can point to the bokeh chart computation and/or share a video of how it crashes.

Sure please see this video

Peek.2024-04-09.17-29.mp4

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 9, 2024

As you can see I'm just getting back to the tree view using the breadcrumb and then crashes

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 9, 2024

and forgot to mention the Bokeh PR to fix the issue which is on branch-3.2, see https://github.com/bokeh/bokeh/pull/13140/files

@robyf70
Copy link
Contributor Author

robyf70 commented May 15, 2024

@LoisRForgeFlow How to we proceed with this PR?

@LoisRForgeFlow
Copy link
Contributor

LoisRForgeFlow commented May 16, 2024

@robyf70 I still think we should stick to the stable version rules and keep it as is in 16.0. However I will make sure that the bokeh version is updated in 17.0. I already pinged the author of the v17 migration asking to update bokeh version there.

In your case, you can use this PR in your instalations to have the newer version and avoid the bug that you are facing. I know is not the ideal solution for you, but there is a risk of impacting many current users of the module if we do this update.

I hope you understand and sorry for the inconveniences 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants