Skip to content

Fix view proxy render window name#3552

Open
Jo-Byr wants to merge 2 commits into
Kitware:masterfrom
Jo-Byr:fix-view-proxy-render-window-name
Open

Fix view proxy render window name#3552
Jo-Byr wants to merge 2 commits into
Kitware:masterfrom
Jo-Byr:fix-view-proxy-render-window-name

Conversation

@Jo-Byr

@Jo-Byr Jo-Byr commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Context

ProxyManager example did not work.

Results

The example is fixed and ViewProxy uses apiSpecificRenderWindow instead of _openGLRenderWindow

Changes

Replace model._openGLRenderWindow by model.apiSpecificRenderWindow in ViewProxy
Fixed argument of render windows' setSize in ProxyManager example

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

@Jo-Byr
Jo-Byr requested a review from finetjul July 16, 2026 10:23
@Jo-Byr Jo-Byr self-assigned this Jul 16, 2026

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain why the name of the variable is problem ?

Why not keeping it protected? (with the _ prefix in the name)

@Jo-Byr

Jo-Byr commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Can you please explain why the name of the variable is problem ?

Why not keeping it protected? (with the _ prefix in the name)

It's not the name directly but the setMacro, there were error thrown due to getApiSpecificRenderWindow not being found.

I removed the '_' to uniformize with other variables but if there is a reason to leave it, I'll put it back.

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please keep the variable "protected" (to prevent infinite loop).

Comment thread Sources/Proxy/Core/ViewProxy/index.js Outdated
Comment thread Sources/Proxy/Core/ProxyManager/example/index.js Outdated
@finetjul

finetjul commented Jul 16, 2026

Copy link
Copy Markdown
Member

It's not the name directly but the setMacro, there were error thrown due to getApiSpecificRenderWindow not being found.

Your commit message should not be "replace openGLRenderWindow by apiSpecificRenderWindow" because

  1. it does not bring additional information than reading the diff
  2. it does not carry the information about what your commit brings to the "user" (i.e. why should I want that commit)

but instead "fix(ViewProxy): add missing function getApiSpecificRenderWindow()" (or something similar).

@Jo-Byr
Jo-Byr force-pushed the fix-view-proxy-render-window-name branch from 7e5714f to 02cf35c Compare July 20, 2026 06:24

@finetjul finetjul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Modify OpenGL and WebGPU RenderWindow setSize
method so that it can accept both an array or 2 arguments
@Jo-Byr
Jo-Byr force-pushed the fix-view-proxy-render-window-name branch from 02cf35c to df8e702 Compare July 21, 2026 06:23
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.

2 participants