Skip to content

Conversation

@adragnevVW
Copy link
Collaborator

Changing the way of how to call Feedback

Moved to IGDTFFixture

@adragnevVW adragnevVW self-assigned this Jun 9, 2025
@adragnevVW
Copy link
Collaborator Author

Hi @AndriiVoitenko,
Sorry for the issue about Feedback, I've moved it to IGDTFFixturePtr.

@AndriiVoitenko
Copy link
Collaborator

@adragnevVW I can not test it, because I am getting endless loop in GdtfFixture::GetFileNodesCount. Have you forget to check VCOM_FAILED on node->GetNextSiblingNode?

@adragnevVW
Copy link
Collaborator Author

Hi @AndriiVoitenko,
I am not understanding you question about the VCOM_FAILED, because we have a check that if we reach the end. Can you send me the file that you are testing to check what if failing, because I tested it with a few files and did not find any issue?

@AndriiVoitenko
Copy link
Collaborator

@adragnevVW
Copy link
Collaborator Author

@AndriiVoitenko I tried to download the file but it redirects me again to the issue

@AndriiVoitenko
Copy link
Collaborator

Robe Lighting@Robin Spiider@2023-07-21 Pattern rotation channel set fix.zip

@AndriiVoitenko
Copy link
Collaborator

you can also look on the implementation
image
if fElement == nullptr, then ppOutNode will be not changes, and while loop here
image
will ends in endless loop

@adragnevVW
Copy link
Collaborator Author

@AndriiVoitenko I achieved to upload the file you sent successfully into Vectorworks, but I got your point and changed it. Thank you, so much for testing it!

@AndriiVoitenko
Copy link
Collaborator

Hi @adragnevVW can you compare CGdtfFixtureImpl::SetAbortCallback and CGdtfFixtureImpl::SetFeedbackCallback
image

I have several questions about it:

  1. Why SetAbortCallback checks fFixtureObject? This function used before reading process started, mean before GDTF file opened for read.
  2. Why CGdtfFixtureImpl::SetFeedbackCallback in normal case returns kVCOMError_FileNotFound?
  3. GdtfObject::SetAbortCallback same as FeedbackDispatcher::SetCallback do not chekcs call back pointer on null pointer, so why CGdtfFixtureImpl::SetFeedbackCallback checks call back pointer and CGdtfFixtureImpl::SetAbortCallback does not?

fTotalCompletedNodes from FixtureFeedback incremented only in GdtfFixture::OnReadFromNode after reading each child node of GDTF Fixture. You know that amount of Nodes in GDTF Fixture is defined over specification and actually does not changes? In other words GdtfFixture::GetFileNodesCount returns always the same count and could be replaced with constant.

And as I told you, I am ok with sending feedback only about reading nodes from GDTF Fixture, but CheckAbort should be called also in other places, at least in reading DMX channels, DMX logical channels and channel functions. Those are usually the biggest part of GDTF file. In example file from issue, exactly reading of those parts takes whole time and abort call back called before reading DMX mode and after reading DMX mode, brings no benefits.

@adragnevVW
Copy link
Collaborator Author

Hi @AndriiVoitenko,
Thanks on the feedback! I've added your suggestions.

@AndriiVoitenko
Copy link
Collaborator

Thanks @adragnevVW . I have another place for CheckAbort: Resolve functions. Currently even if you aborting reading from node, GdtfFixture::ResolveAllReferences will be still called and some of Resolve functions take some time. Please add check abort also into Resolve functions explicitly into GdtfFixture::ResolveDMXModeMasters

@adragnevVW
Copy link
Collaborator Author

@AndriiVoitenko It's added, thank you too!

@AndriiVoitenko
Copy link
Collaborator

I mean all resolve function and not only ResolveDMXModeMasters.

ResolveDmxModeRefs, ResolveAttribRefs, CheckForMissingModeMasters also have loops in and would also take some amount of time.

@adragnevVW
Copy link
Collaborator Author

Hi @AndriiVoitenko,
I've added your suggestions. I will also check for more possible loops

Copy link
Collaborator

@AndriiVoitenko AndriiVoitenko left a comment

Choose a reason for hiding this comment

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

Looks good, thank you @adragnevVW

@adragnevVW adragnevVW merged commit 0a686e3 into master Jun 13, 2025
1 check passed
@adragnevVW adragnevVW deleted the Updating-Feedback-on-reading-Fixtures branch June 13, 2025 12:47
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.

4 participants