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

[NUI] Search upper parents until registered object is found #5377

Closed
wants to merge 2 commits into from

Conversation

jmmhappy
Copy link
Contributor

@jmmhappy jmmhappy commented Jul 7, 2023

If a component's parent is not registered on NUI but still exists on DALi side, GetParent() on the object returns null.
This makes it impossible to reach to the component's NUI parent.

For example, when a child is Scene3D.Camera and its parent is SceneView, child.GetParent() always returns null(SceneView.mRootLayer not registered on NUI), thus SceneView is unreachable from Scene3D.Camera.

This patch implements recursive search on dali parents when NUI child's InternalParent is null.

Copy link
Contributor

@huiyueun huiyueun left a comment

Choose a reason for hiding this comment

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

LGTM

BaseHandle baseHandle = Registry.GetManagedBaseHandleFromNativePtr(parent.Handle);
InternalParent = baseHandle;

return baseHandle as Container;
Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to call DeleteBaseHandle for cPtrParent before return it. See how Tizen.NUI.Extensions.GetInstanceSafety<> implemented.

Suggested change
return baseHandle as Container;
Interop.BaseHandle.DeleteBaseHandle(parent);
parenr = new HandelRef(null, IntPtr.Zero);
return baseHandle as Container;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comment. I think we forgot to Delete(parent). But I wonder how the last line(:280) matters. Why would you put another new (zero pointer handle) to this local variable (parent)?

I tried to search Tizen.NUI.Extensions.GetInstanceSafety<> function but could not find it.

@jmmhappy jmmhappy closed this Jul 12, 2023
@dongsug-song
Copy link
Contributor

@jmmhappy
안녕하세요.
아까 구두로 설명 드린 내용 정리 드립니다.

  • View 의 GetParent() 는 public API 라서 behavior 가 바뀌면 기존 앱들에 영향이 있으므로 수정하지 않는 것을 권고 드립니다.
  • 만약 바꿔야 한다면 정식으로 ACR 을 해야 하고 이것을 쓰는 앱 개발자분들에게 변경 사항에 대해서 공지를 해야 하고 파생되는 문제점에 대해서 모두 대응을 해야 됩니다.
  • Scene3DView 에 hidden API 로 다른 이름의 메소드 로 추가하시는게 좋을 것 같습니다. 아까는 프로퍼티인줄 알고 new 로 overriding 을 하라고 말씀드렸는데, 이것은 메소드에는 틀린 내용이고 같은 이름이면 argument 를 추가하시거나 이름을 변경해서 추가하시면 될 것 같습니다.
  • 그리고, Registry 에서 handle 을 가져오는것은 user 가 NUI 단에서 new 로 object 를 생성하는 경우에 가능합니다. 만약 여기에 camera 가 dali 에서 default 로 이미 생성된 것이라면 Registry 에서 핸들을 가져오면 항상 null 이 나옵니다. 따라서, 사용자가 new 로 만든것이 아니라면 nui 에 매칭되는 object class 를 찾아서 swigcptr 을 직접 넣어주셔야 합니다. 예를 들면, IntPtr cPtrParent = Interop.Actor.GetParent(SwigCPtr); 에서 dali 에서 받아온 IntPtrcPtrParentCamera c = new Camera(cPtrParent, true); 이런식으로 nui 껍데기 object 를 만들어서 IntPtr 을 set 해 줘야 합니다.

확인 부탁드립니다. 감사합니다.

@jmmhappy jmmhappy deleted the ask_parent branch August 3, 2023 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants