Skip to content

ISSUE-2082: Add repeat support for bindings API - #3318

Open
CPerry26 wants to merge 13 commits into
f3d-app:masterfrom
CPerry26:issue-2082
Open

ISSUE-2082: Add repeat support for bindings API#3318
CPerry26 wants to merge 13 commits into
f3d-app:masterfrom
CPerry26:issue-2082

Conversation

@CPerry26

@CPerry26 CPerry26 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Add repeat support for bindings API. If you set repeat to false, it will ignore repeated key presses from holding down the key. Otherwise it will keep applying the binding behavior (i.e. increase light intensity). Please note this work requires VTK changes.

Issue ticket number and link if any

#2082

Checklist for finalizing the PR

  • I have performed a self-review of my code
  • I have added tests for new features and bugfixes
  • I have added documentation for new features
  • If it is a modifying the libf3d API, I have updated bindings
  • If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

AI Disclosure

  • I did not use AI to generate any of the content of that pull request
  • I used AI to generate code in that pull request, if yes please disclose which part of the code was generated and with which model.
  • ...

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

@CPerry26

Copy link
Copy Markdown
Contributor Author

@mwestphal @snoyer Here's a draft of changes. I didn't add any tests yet because I wanted to make sure this is the right path, and that any defaults to existing bindings we want applied are setup first. Let me know what you think!

const BindingCommands binding = commandsIt->second;

for (const std::string& command : binding.CommandVector)
if (binding.Repeat || rwi->GetRepeatCount() <= 1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Went with <= here in case VTK can support starting the repeat count at 0. Can switch the logic if we'd like.

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.

will need updated once VTK changes indeed.

@github-actions

Copy link
Copy Markdown

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: c, python, java, webassembly.

@mwestphal
mwestphal self-requested a review June 30, 2026 13:27

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

looks nice

@mwestphal mwestphal linked an issue Jul 5, 2026 that may be closed by this pull request
Comment thread testing/recordings/TestInteractionBindingsRepeatFalse.log Outdated
@CPerry26
CPerry26 marked this pull request as ready for review July 28, 2026 21:53
@CPerry26
CPerry26 requested a review from a team as a code owner July 28, 2026 21:53
@CPerry26

Copy link
Copy Markdown
Contributor Author

@mwestphal Still need the VTK bump first, but would love to get your thoughts on:

  1. Should we default any bindings to repeat true?
  2. Should I update the bindings in this PR?

@mwestphal

mwestphal commented Jul 29, 2026

Copy link
Copy Markdown
Member

Should we default any bindings to repeat true?

Actually no, default should be false, only a select few should be true.

Should I update the bindings in this PR?

Yes

Comment thread library/public/interactor.h Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOff.log.in Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOn.log.in Outdated
Comment thread library/testing/TestSDKBindingRepeat.cxx Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOff.log.in Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOn.log.in Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOff.log Outdated
Comment thread testing/baselines/TestSDKBindingRepeatOff.png
@CPerry26
CPerry26 requested a review from mwestphal July 30, 2026 12:10
Comment thread testing/recordings/TestSDKBindingRepeatOff.log Outdated
Comment thread testing/recordings/TestSDKBindingRepeatOn.log Outdated
Comment thread python/F3DPythonBindings.cxx
Comment thread java/Interactor.java Outdated
Comment thread java/Interactor.java
Comment thread c/interactor_c_api.h
@CPerry26
CPerry26 requested a review from mwestphal August 1, 2026 13:25
KeyPressEvent 1591 863 1 80 9 P 0
KeyPressEvent 1591 863 1 80 10 P 0
KeyReleaseEvent 1591 863 1 80 1 P 0

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.

Suggested change

KeyPressEvent 1591 863 1 80 9 P 0
KeyPressEvent 1591 863 1 80 10 P 0
KeyReleaseEvent 1591 863 1 80 1 P 0

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.

Suggested change

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

last changes, but needs the VTK uniformization of behavior before merge I think.

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.

Add a repeat flag in the binding API

3 participants