-
Notifications
You must be signed in to change notification settings - Fork 254
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.Scene3D] Add capture to Scene3D.SceneView #6271
Conversation
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::captureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::capturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.SceneView::Dispose(Tizen.NUI.DisposeTypes)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::captureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::capturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
src/Tizen.NUI.Scene3D/src/public/Controls/CaptureFinishedEventArgs.cs
Outdated
Show resolved
Hide resolved
src/Tizen.NUI.Scene3D/src/public/Controls/CaptureFinishedEventArgs.cs
Outdated
Show resolved
Hide resolved
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment at demo app
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
/// <returns> capture id that id unique value to distinguish each requiest.</returns> | ||
// This will be public opened after ACR done. (Before ACR, need to be hidden as Inhouse API) | ||
[EditorBrowsable(EditorBrowsableState.Never)] | ||
public int Capture(Scene3D.Camera camera, Vector2 size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dali 구조와 상관 없이 순수 API사용자 관점에서 Capture API의 위치는 camera객체가 갖고 있어야 하는게 아닐까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
특정 행위에 대한 결과물이 비동기적으로 전달되는 경우에는
C#에서는 이벤트보다는 Task를 사용하는 것이 좋기는 합니다.
public int Capture(Scene3D.Camera camera, Vector2 size) | |
public Task<ImageUrl> Capture(Scene3D.Camera camera, Vector2 size) |
이런 API가 제공되면 앱 개발자는 이렇게 단순하게 코드 작성이 가능해 집니다.
var url = await sceneView.Caputre(camea, new Vector2(100, 100));
imageView.ResourceUrl = url.ToString();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageUrl class 는 UI Thread 에서만 생성되어야하는 class 이기 때문에 Task 로 만들 수 없습니다. ImageUrl class 는 외부 요인에 의해 생성된 이미지요소(ex : Texture, NativeImage 등,)의 lifecycle을 관리하기위한 class 이기 때문입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scene3D 개발자 관점에서, 카메라를 가져오고 설정하는 API 는 모두 SceneView에 있는데 Capture 만 Camera API 로 빠져있는 것이 더 어색해 보입니다.
하나의 Camera 가 동시에 여러 SceneView 에 속해있을 수도 없고, 또한 Camera 자기 자신은 자기가 속해있는 SceneView 도 모르는 상황으로, 정말 Camera 관련 속성들만 제어/관리 하고 있습니다. 이런 상태에서 SceneView 에 의존성이 생기는 API 하나를 추가하는 것은 별로 좋은 구조로 보이지 않습니다.
View.SetRenderEffect(BackgroundBlurEffect) 처럼 RenderEffect 에 대한 구현을 Vjew 가 아니라 RenderEffect 에서 설정하는 것처럼 느껴집니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dali 구조와 상관 없이 순수 API사용자 관점에서 Capture API의 위치는 camera객체가 갖고 있어야 하는게 아닐까요?
말씀하신 형태도 API관점에서 좋은 방법이라고 생각합니다만, 몇가지 문제가 있는데요.
예를들어, Camera가 화면을 촬영하기 위해서 필요한 몇가지 속성 중에 종횡비가 있습니다.
다른 3D 엔진의 경우에는 Window종횡비가 곧 3D Scene의 종횡비이기 때문에 Camera는 대부분의 경우 Window 데이터에만 접근할 수 있으면 되는데요.
NUI 의 SceneView는 2D Content에 들어가는 Component이므로 하나의 Scene에 여러개의 SceneView를 추가할 수 있으며, 여러 SceneView가 하나의 Camera를 돌려 쓸 수 있습니다.
SceneView별로 카메라를 위한 속성이 다르므로, 위에서 말씀드린 종횡비를 포함하여 Capture를 위해 필요한 속성들이 SceneView에 종속적으로 동작하기 때문에, SceneView에서 Capture를 컨트롤 하는 것이 적합합니다.
그 외에도 차치하신 DALi 구조적인 문제로서도 Camera에서 Capture를 제공하기 위해서는 다양한 고통이 필요한데요.
NUI.Scene3D에서는 대부분의 씬을 위한 기능이 SceneView를 통해 제공되고 있기도 하고, 어쨌거나 해당 SceneView에 포함된 Scene의 렌더링 결과를 얻어내고자 하는 API가 SceneView에 있는 것이 큰 문제가 되지는 않을 것이라고 생각됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int ret = DummyAsync().Result;
이걸 쓰셨으니까 그렇습니다. 저 속성은 동기적으로 (thread를 블럭하여) 결과를 기다립니다.
제대로된 사용법으로 사용하면 블럭이 안됩니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가이드 주신대로 여러가지 시도를 해본 결과 OnTouch 는 정상적으로 끝나면서 main thread 에서 발생한 이벤트도 정상적으로 받는 것을 확인했습니다.
private bool OnTouch(object s, View.TouchEventArgs e)
{
if (e.Touch.GetState(0) == PointStateType.Down)
{
Tizen.Log.Error("NUI",$"Before ASDF\n");
ASDF();
Tizen.Log.Error("NUI",$"End ASDF\n");
}
return true;
}
private async void ASDF()
{
Tizen.Log.Error("NUI",$"Before await\n");
var task = DummyAsync();
int ret = await task;
Tizen.Log.Error("NUI",$"End await {ret}\n");
}
결국은 async 라는 태그가 달린 별도의 함수를 앱에서 알아서 잘 구현해야만 하고, await 이후 ImageUrl 을 받은 사용자가 무엇을 어떻게 할지는 저희가 아직은 신경 안써도 된다고 이해했습니다.
그렇다면 말씀해주신대로 이후의 작업은 C# 언어 개발자의 숙련도 문제이지, Framework 의 문제가 아닌 것으로 이해되었습니다.
if (e.Success)
{
ret.SetResult(e.CapturedImageUrl);
}
소소하게 이 부분에 대해서, 우선 e.CapturedImageUrl 은 cMemOwned 가 false 인 상태로 만들어졌기 때문에, 이 상태 그대로 SetResult 에 넣으면 안되고, Registry 에 등록을 한번 해야지 정상적인 사용이 가능할 것으로 보입니다. 승호님께서 구현하실 때 참고바랍니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그런데 다른 질문이 있는데요,
예를 들어 FileIO 에 걸려있는 아무개 Task 라고 하더라도 유저가 원한다면 main loop 에 block 을 걸어서 FileIO 가 끝날때 까지 강제로 기다리게 하는 방법들이 잘 고려가 되어있을 것으로 예상되는데요...
지금의 Capture 처럼 main loop 에 dependency 가 있는 Task 의 경우에는 앱에서 동기적으로 task complete 결과를 기다리는 코드를 사용한 경우 무조건 dead lock 에 걸리게 될 것인데
이러한 사항이 일반적인 것이어서 따로 주석을 달지 않아도 되는건지, 아니면 함수에서 주석으로 적어놓기만 하면 되는건지 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저희가... event로 뭔가를 제공할때도..
main loop에서 Sleep, 또는 while로 점유한체 대기하면 event가 오지 않을 것인데
이러한 사항을 remarks에 적지 않는것과 동일한게 아닐까요?
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
Internal API ChangedAdded: 6, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
|
||
Interop.SceneView.CaptureFinishedDisconnect(GetBaseHandleCPtrHandleRef, captureFinishedEventCallback.ToHandleRef(this)); | ||
NDalicPINVOKE.ThrowExceptionIfExistsDebug(); | ||
captureFinishedEventCallback = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SceneView CaptureFinished 이벤트가 오기 전에 SceneView 자기 자신이 Dispose() 될 수 있으니, 이 경우 CaptureAsync 를 기다리는 유저한테 실패정보를 알려주기 위해서 TaskCompletionSource<ImageUrl>
을 private 변수로 만들고, 이 Dispose 타이밍에 SetResult() 를 해줘야 할 것 같습니다.
CaptureFinished 콜백 형태로만 제공한 경우에는 지금처럼 Dispose 된 경우에 별도로 앱한테 DIspose 여부를 알려줄 필요가 없었을텐데...Task<> 로 제공해버린 경우 SetResult 가 오기 전까지 await 를 걸어버린 함수가 계속 남아있을 것이기 때문에 (아마도?) 강제로 SetResult 를 해줘야만 한다..라고 이해를 하고있는데요..
이렇게 Dispose 가 되는 도중 (~= Disposed 가 아직 false 이지만 기본적인 리소스들이 한창 해제가 되고있는 와중에) 유저가 추가적으로 무언가 작업을 할 수 있도록 하는 여지를 만드는게 과연 좋은 일인지 모르겠습니다.
유저가 이 Dispose 로 인해서 Capture 가 취소된 경우를 별도로 처리할 수 있게 하기 위해서, 결국 Capture Result 의 종류를 3가지 이상 (성공 / 실패 / Dispose 로 인한 취소 등) 을 만들어야하고, 특히 Dispose 로 인한 취소가 결과물로 넘어간 경우에는 SceneView 에 접근하는 어떤 동작도 구현해서는 안된다는 제약사항을 주석으로만 남겨야하는데... 앱 개발자들이 이 동작을 잘 납득하고 그대로 따라줄지는 모르겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
특히 이 "Dispose 로 인한 취소" 같은 경우에는 CaptureFinished 콜백을 사용하는 경우에는 필요없는 정보이기 때문에, CaptureAsync 에서만 사용되게 될텐데... 앞으로 Task<> 형태로 return 하는 API 를 계속해서 추가할 때마다 '실제 return 정보 + Dispose 로 인한 취소 여부' 정보를 포함한 struct 를 새로 정의해서 Task 의 return 타입으로 만들어나가면 되는건지.. 의견을 구합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capture Finished전에 SceneView SceneOff하는 부분에 대해서 Fail 올리는 것으로 하겠습니다.
이 문제 겸사겸사 찝찝했던 것을 해결하는 겸사겸사 SceneOff 타이밍이나 SceneView 소멸 시점에 실패 콜백을 올려주도록 수정 할 계획입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인해보니 SceneOff때는 이미 남은 캡쳐 요청들에 대해서 실패 콜백을 올려주고 있었습니다.
{ | ||
CaptureFinishedEventArgs e = new CaptureFinishedEventArgs(); | ||
e.CaptureId = captureId; | ||
e.CapturedImageUrl = new ImageUrl(capturedImageUrl, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImageUrl의 ownership이 없는건가요? 없으면 안될것 같습니다. event핸들러에서 args로부터 받은 데이터를 event 핸들러 밖에서 사용하지 말라는 법이 없어서, 위에서 논의한 Task
리턴과 상관 없이 고쳐야할 것 같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정했습니다.
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
ImageUrl ret = Registry.GetManagedBaseHandleFromNativePtr(capturedImageUrl) as ImageUrl; | ||
if (ret != null) | ||
{ | ||
NUI.Interop.BaseHandle.DeleteBaseHandle(new HandleRef(this, capturedImageUrl)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inputed capturedImageUrl
Don't have ownership of memory.
We should not delete basehandle here.
NUI.Interop.BaseHandle.DeleteBaseHandle(new HandleRef(this, capturedImageUrl)); |
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
/// </summary> | ||
// This will be public opened after ACR done. (Before ACR, need to be hidden as Inhouse API) | ||
[EditorBrowsable(EditorBrowsableState.Never)] | ||
internal event EventHandler<CaptureFinishedEventArgs> AsyncCaptureFinished |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이게 두개 있을 필요는 없어요...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이벤트는 그냥 CaptureFinished
하나를 같이 사용해도 될 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CaptureFinished 콜백에 함수를 등록해놓고, CaptureAsync 를 사용한 사용자가 있는 경우, Capture API 를 사용하지도 않았는데 CaptureFinisehd 콜백이 날아가게 됩니다.
앱 개발자가, 자신이 요청한 CaptureId 초기값을 예쁜 값을 잘 집어넣거나, Capture API 를 몇번 호출했는지 여부를 일일히 기억했다가 Valid 한 CaptureFinished 콜백이 온 경우에 이 값을 1씩 까내리는 식으로, CaptureFinished 콜백에서 적절한 예외처리가 반드시 필요하다고 주석에다가 주저리주저리 명시하는 것보다, 아얘 이벤트를 쏘지 않는 것으로 나가는게 어떨까 생각했씁니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100번 양보해서 이렇게 하면 1개의 signal을 사용해도 원하시는 결과대로 동작할 듯합니다.
event EventHandler<CaptureFinishedEventArgs> _asyncCaptureFinished;
event EventHandler<CaptureFinishedEventArgs> AsyncCaptureFinished
{
add
{
if (_asyncCaptureFinished == null)
{
CaptureFinished += dummy;
}
_asyncCaptureFinished += value;
}
remove
{
_asyncCaptureFinished -= value;
if (_asyncCaptureFinished == null)
{
CaptureFinished -= dummy;
}
}
}
void dummy(object sender, CaptureFinishedEventArgs e) {}
private void OnCaptureFinished(IntPtr data, int captureId, IntPtr capturedImageUrl)
{
.....
if (asyncCaptureIds.Contains(e.CaptureId))
{
_asyncCaptureFinished?.Invoke(this, e);
}
else
{
captureFinishedEventHandler?.Invoke(this, e);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 internal은 잘못된 접근 제한자 같습니다.
어차피 이 클래스 내에서만 접근하는 요소이기 때문에 private가 더 적합합니다. (제가 작성한 예제들은 접근제한자를 생략했는데 이 경우 private와 동일하게 취급됩니다.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한... 지금 구현은 OnCaptureFinished
로 두 이벤트에 동일한 시그널 핸들러를 사용하고 있는데,
그러면
CaptureFinished
에 이벤트 핸들러를 등록하고CaputreAsync
를 호출한 상황에서
OnCaptureFinished
는 두번 호출됩니다.
왜냐, 1번에 의해서 등록된 시그널 핸들러
2번에 의해서 등록된 시그널 핸들러.
여기까지는 동의가 되시나요?
그리고 결과가 async캡춰의 결과이기 때문에
asyncCaptureFinishedEventHandler
를 호출해주고. 이것에 의해서
if (e.CaptureId == captureId)
{
asyncCaptureIds.Remove(e.CaptureId);
Tizen.Log.Error("NUI", $"cnt : {asyncCaptureIds.Count}\n");
if (e.CapturedImageUrl != null)
{
ret.SetResult(e.CapturedImageUrl);
}
else
{
ret.SetException(new InvalidOperationException("Fail to Capture"));
}
}
이 코드가 수행되고, asyncCaptureIds에서 id가 삭제.
그리고 두번째 OnCaptureFinished
가 호출되고 id가 asyncCaptureIds에 없기 때문에 captureFinishedEventHandler가 호출 그렇지만 그 결과는 자신이 요청하지 않은 이벤트가 자신도 모르게 발생하는 async요청의 결과!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한... 지금 구현은
OnCaptureFinished
로 두 이벤트에 동일한 시그널 핸들러를 사용하고 있는데, 그러면
CaptureFinished
에 이벤트 핸들러를 등록하고CaputreAsync
를 호출한 상황에서
OnCaptureFinished
는 두번 호출됩니다. 왜냐, 1번에 의해서 등록된 시그널 핸들러 2번에 의해서 등록된 시그널 핸들러.여기까지는 동의가 되시나요?
그리고 결과가 async캡춰의 결과이기 때문에
asyncCaptureFinishedEventHandler
를 호출해주고. 이것에 의해서if (e.CaptureId == captureId) { asyncCaptureIds.Remove(e.CaptureId); Tizen.Log.Error("NUI", $"cnt : {asyncCaptureIds.Count}\n"); if (e.CapturedImageUrl != null) { ret.SetResult(e.CapturedImageUrl); } else { ret.SetException(new InvalidOperationException("Fail to Capture")); } }이 코드가 수행되고, asyncCaptureIds에서 id가 삭제. 그리고 두번째
OnCaptureFinished
가 호출되고 id가 asyncCaptureIds에 없기 때문에 captureFinishedEventHandler가 호출 그렇지만 그 결과는 자신이 요청하지 않은 이벤트가 자신도 모르게 발생하는 async요청의 결과!!
네네 안그래도 이 부분이 문제가 돼서 수정중에 마무리 못하고 퇴근했는데 말씀하신 방법을 고려해 보겠습니다.
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
public async Task<ImageUrl> CaptureAsync(Scene3D.Camera camera, Vector2 size) | ||
{ | ||
TaskCompletionSource<ImageUrl> ret = new TaskCompletionSource<ImageUrl>(); | ||
int captureId = 0; | ||
EventHandler<CaptureFinishedEventArgs> handler = (s, e) => | ||
{ | ||
if (e.CaptureId == captureId) | ||
{ | ||
asyncCaptureIds.Remove(e.CaptureId); | ||
Tizen.Log.Error("NUI", $"cnt : {asyncCaptureIds.Count}\n"); | ||
if (e.CapturedImageUrl != null) | ||
{ | ||
ret.SetResult(e.CapturedImageUrl); | ||
} | ||
else | ||
{ | ||
ret.SetException(new InvalidOperationException("Fail to Capture")); | ||
} | ||
} | ||
}; | ||
|
||
AsyncCaptureFinished += handler; | ||
captureId = Interop.SceneView.Capture(SwigCPtr, camera.SwigCPtr, Vector2.getCPtr(size)); | ||
asyncCaptureIds.Add(captureId); | ||
try | ||
{ | ||
return await ret.Task; | ||
} | ||
finally | ||
{ | ||
AsyncCaptureFinished -= handler; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id를 맵으로 관리할꺼면 이렇게 await를 사용할 필요가 없습니다.
public async Task<ImageUrl> CaptureAsync(Scene3D.Camera camera, Vector2 size) | |
{ | |
TaskCompletionSource<ImageUrl> ret = new TaskCompletionSource<ImageUrl>(); | |
int captureId = 0; | |
EventHandler<CaptureFinishedEventArgs> handler = (s, e) => | |
{ | |
if (e.CaptureId == captureId) | |
{ | |
asyncCaptureIds.Remove(e.CaptureId); | |
Tizen.Log.Error("NUI", $"cnt : {asyncCaptureIds.Count}\n"); | |
if (e.CapturedImageUrl != null) | |
{ | |
ret.SetResult(e.CapturedImageUrl); | |
} | |
else | |
{ | |
ret.SetException(new InvalidOperationException("Fail to Capture")); | |
} | |
} | |
}; | |
AsyncCaptureFinished += handler; | |
captureId = Interop.SceneView.Capture(SwigCPtr, camera.SwigCPtr, Vector2.getCPtr(size)); | |
asyncCaptureIds.Add(captureId); | |
try | |
{ | |
return await ret.Task; | |
} | |
finally | |
{ | |
AsyncCaptureFinished -= handler; | |
} | |
} | |
Dictionary<int, TaskCompletionSource<ImageUrl>> asyncCatpureIds = new(); | |
public Task<ImageUrl> CaptureAsync(Scene3D.Camera camera, Vector2 size) | |
{ | |
void Handler(object _, CaptureFinishedEventArgs e) | |
{ | |
if (asyncCatpureIds.TryGetValue(e.CaptureId, out var tcs)) | |
{ | |
try | |
{ | |
if (e.CapturedImageUrl != null) | |
{ | |
tcs.SetResult(e.CapturedImageUrl); | |
} | |
else | |
{ | |
tcs.SetException(new InvalidOperationException("Fail to Capture")); | |
} | |
} | |
finally | |
{ | |
CaptureFinished -= Handler; | |
asyncCatpureIds.Remove(e.CaptureId); | |
} | |
} | |
}; | |
CaptureFinished += Handler; | |
var captureId = Interop.SceneView.Capture(SwigCPtr, camera.SwigCPtr, Vector2.getCPtr(size)); | |
TaskCompletionSource<ImageUrl> ret = new TaskCompletionSource<ImageUrl>(); | |
asyncCatpureIds.Add(captureId, ret); | |
return ret.Task; | |
} |
그리고 이벤트를 둘로 분리하신 이유가 아마도 async로 호출한 결과가 event로 오는것이 이상하다고 생각해서 그런건가요? 근데, 어차피 이벤트 핸들러를 작성하는 입장에서는 CaptureId로 자신이 기달리는 결과인지 확인하기 때문에 상관 없는게 아닐까요? 저는 이런식으로 event가 두개 생기는것은 원치 않습니다.
DALI에까지 Signal객체를 두개씩이나 만들면서 event기반 Task기반 두 API를 제공할 필요는 없는것 같습니다.
제가 둘다 제공하자고 했던 입장은 이미 기존 event기반 API에서 코드 조금만 추가하면 Task기반 API추가가 가능했기에 의견을 드렸던것입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한, Event방식으로 사용하는 사람은 event방식으로만 사용할텐데 (Async방식의 API호출 자체를 안할텐데....)
event에 async결과가 섞여 들어가는 상황이 흔하지 않는 상황으로 보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
또한, Event방식으로 사용하는 사람은 event방식으로만 사용할텐데 (Async방식의 API호출 자체를 안할텐데....)
두 방식 모두 평범하게 사용 가능한 API 들이고, 따라서 두 방식을 혼용해서 사용하는 사용자가 나타날 수도 있습니다. 플랫폼 개발하는 입장이서 이러한 사용자를 무시할 수 없습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CaptureId로 자신이 기달리는 결과인지 확인하기 때문에 상관 없는게 아닐까요?
앱 개발자가 Capture 를 한번도 보내지 않은 상태에서 앱 개발자가 저장해놓은 CaptureId 가 어떤 값이 될지 알 수 없습니다. 초기값으로 적당히 셋팅해놓은 0 이라는 Id 값이 예를 들어서 CaptureAsync 에서 사용한 Id 와 동일할 수도 있고, 이 경우 CaptureFinished 콜백에서 앱 개발자 본인도 모르게 Capture 관련 로직이 수행되게 됩니다. 이 로직이 CaptureAsync 로 리턴된 Task 와 상충될 수도 있기 때문에 마냥 무시할 수 없다고 생각합니다.
이런식으로 event가 두개 생기는것은 원치 않습니다.
앱 개발자도 자신이 요청하지 않은 이벤트가 자신도 모르게 Invoke 되는 것을 원치 않을것이라 생각했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API두개가 존재하는 이상 두가지 방법을 쓰지 않는다는 보장을 하기는 어렵습니다.
앱 개발을 혼자 하는 것은 아니다보니 더더욱 통일되지 않은 개발법을 사용하더라고요.
그리고, Async로 요청하는 경우에는 CaptureId가 애초에 사용자에게 노출되지 않기 때문에 콜백이 호출 되어도 의미가 없기 때문에 보내지 않는 편이 나을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dictionary를 사용하면 좀 더 재미있게 처리할 수가 있군요. 감사합니다.
if (ret == null) | ||
{ | ||
ImageUrl imageUrl = new ImageUrl(capturedImageUrl, false); | ||
ret = new ImageUrl(NUI.Interop.ImageUrl.NewImageUrl(imageUrl.SwigCPtr), true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... 이거는 Dali에서 리턴할때부터 ownership이 있는체로 주면 안될까요? 어차피 매번 여기에서 새로운 handle객체를 만드는것보다 나을것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재는 아마 Signal객체의 callback에서 받는 parameter 타입이 ImageUrl의 래퍼런스로 되어 있어서, 이게 자동적으로 signal을 호출한 stack을 빠져나가면 자동으로 삭제되는것 같고, 이런 방식이 C++만 사용하는 상황에서는 매우 합당해 보입니다.
그러나 해당 시그널이 C++ 코드를 통해서만 사용되는 시그널도 아니며,
다른 대부분의 경우 binder를 통해 C#으로 객체가 전달 될때는 새로운 BaseHandle
객체를 생성해서 그 폰인터를 전달하는 방식이 거의 대부분인데 여기서만 예외적으로 소유권이 없는 BaseHandle객체의 래퍼런스(결국 포인터로 변환되는...하지만 소유권이 없는)로 전달하고 있어서 아마 나중에라도 실수가 발생하지 않을지 걱정입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 대부분의 경우 binder를 통해 C#으로 객체가 전달 될때는 새로운 BaseHandle 객체를 생성해서
현재 NUI 는 Native 에서 Callback 이 올라오는 많은 경우에 대해서 새로운 BaseHandle 객체를 생성하고있지 않습니다. 예를 들어서 OnKeyEvent(IntPtr view, IntPtr keyEvent)
의 입력인지인 view
와 keyEvent
둘 다 Native 가 ownership 을 가진 BaseHandle 객체이고, e.Key = Tizen.NUI.Key.GetKeyFromPtr(keyEvent);
라는 코드를 통해서 ownership 이 없는 Key
객체를 event argument 로 넣어주고 있습니다.
View 랑 Window 의 모든 이벤트들이 이렇게 구현되어있어서 이것이 일반적인 구현법이라고 생각했는데, 지금 보니 FocusManager 랑 WebView 같은 경우에는 ownership 이 있는 객체를 argument 로 사용하고있네요. 상황에 따라서 다른가봅니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
새로운 BaseHandle 객체를 생성하기 위해서는 dali-csharp-binder 에서 이벤트를 한번 먹어버리고, C# 만을 위한 event hander callback 을 argument 로 받아서 처리를 해야하는데 (CSharp_Dali_WebView_RegisterPageLoadErrorCallback
등 WebView 관련 이벤트들이 전부 입력인자로 C# 콜백을 집어넣도록 구현되어있습니다.) 이 방식대로 수정할 수 잇는지 검토해보겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FocusManager 와, VisibilityChanged 의 argment 로 사용된 View 들은 전부 NUI 에서 생성되었다는 가정 하에서 돌아가는 코드들이고, 콜백으로 넘어오는 인자 자체는 ownership 이 없는 pointer 가 넘어오고 있습니다.
WebView 처럼 bridge 역할을 하는 함수를 dali-csharp-binder 에서 추가로 구현해도 되겠지만, 개인적으로는 WebView 가 특이케이스고, NUI 플랫폼을 개발하는 입장에서는 일관된 정책 (=Native 콜백에서 직접 넘어오는 객체는 모두 ownership이 없다) 을 가정하고 구현하는 것이 오히려 실수를 방지하고 헷갈리지 않을 것이라고 생각됩니다.
무엇보다 dali-csharp-binder 에서 코드 1줄이면 signal 이 연결되는데, 이 코드를 해체하고 별도의 bridge 역할의 함수를 만드는 것이 심적으로 부담이 됩니다. :D
GENERATE_SCENE_VIEW_SIGNAL(void (*)(Dali::Scene3D::SceneView, int32_t, Dali::Toolkit::ImageUrl const&), CaptureFinishedSignal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게하고 emit하는데서 new Dali::Toolkit::ImageUrl(imgurl)
로 보내줘도 되지 않나요?
다만 그렇다면 받는데서 제대로 delete안하면 메모리 릭의 위험이 있는거지만...
GENERATE_SCENE_VIEW_SIGNAL(void (*)(Dali::Scene3D::SceneView, int32_t, Dali::Toolkit::ImageUrl*), CaptureFinishedSignal)
암튼 이해는합니다. 그래서 바뀌지 않아도 되지만, 확실히 소유권에 대한 명시를 할 수 있는 방법이 binder API에 있었으면 좋겠습니다.
네이밍을 바꾸던지.. 구분할 수 있도록...
{ | ||
CaptureFinishedEventArgs e = new CaptureFinishedEventArgs(); | ||
e.CaptureId = captureId; | ||
ImageUrl ret = Registry.GetManagedBaseHandleFromNativePtr(capturedImageUrl) as ImageUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Registry에서 검사가 필요한 상황이 있을까요?
Registry에서 검사하는 이유는 이미 c#에 같은 object를 가르키는 객체가 존재할 경우를 대비해서 검색하는건데,
capture된 결과의 ImageUrl의 경우 매번 캡춰할 때마다 새롭게 생성되는 객체가 아닌가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분에 대해서도 고민을 했었는데요... 앞으로 미래의 dali scene3d 개발자의 마음이 변해서 ImageUrl class 를 재사용해서 (정확히는 ImageUrl 이 소유하고 있는 Texture 를 재사용해서) CaptureFinished argument 로 넘기도록 동작을 수정할 수도 있기 때문에 Registry 검사를 추가했습니다.
현재 구현상으로는 ImageUrl 을 매번 새로 생성하고 있지만 앞으로도 계속 그럴 것이라는 보장을 할 수 없고, 그런 미래의 상황이 왔을 때 OnCaptureFinished 콜백의 구현부를 수정하지 않고 dali 만의 수정사항으로 문제를 해결하고 싶었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... 그래야 하는 이유가 있나요? 아니면... 그렇게 하지 말아야할 이유는 많지 않나요? ImageUrl이 언제까지 그 시점에 capture된 이미지를 담고 있어야 한다면... 저는 ImageUrl이 삭제 되지 않는 한 계속 그 캡춰된 이미지를 담고 있어야 한다고 생각합니다.
그런데 막 이게 아무런 기준이 없이 어느 순간에 다름 이미지로 바뀌어 있으면 매우 당황할 것 같습니다.
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
NDalicPINVOKE.ThrowExceptionIfExists(); | ||
|
||
e.CaptureId = captureId; | ||
e.CapturedImageUrl = imageUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이게 아마 BaseHandle에서 ==
/!=
연산자에 대한 overloading이 되어 있어서 큰 문제는 없지만 (사용할 때, == null
로 검사가 가능), 그렇지만 그럼에도 불구하고 null검사를 하지 않고 그냥 메소드를 호출하는 경우 NullReferenceException
이 아닌 Dali가 보내는 body가 없다는 exception을 받게 되어서 이상한 것 같습니다.
이런식으로 고치면 어떨까요?
e.CapturedImageUrl = imageUrl; | |
e.CapturedImageUrl = imageUrl.HasBody() ? imageUrl : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 방법인 거 같습니다.
Signed-off-by: Seungho Baek <[email protected]>
Internal API ChangedAdded: 7, Removed: 0, Changed: 0Added+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.Scene3D.CaptureFinishedEventArgs
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CaptureId()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ Tizen.NUI.ImageUrl Tizen.NUI.Scene3D.CaptureFinishedEventArgs::CapturedImageUrl()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Void Tizen.NUI.Scene3D.CaptureFinishedEventArgs::.ctor()
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Int32 Tizen.NUI.Scene3D.SceneView::Capture(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Threading.Tasks.Task`1<Tizen.NUI.ImageUrl> Tizen.NUI.Scene3D.SceneView::CaptureAsync(Tizen.NUI.Scene3D.Camera,Tizen.NUI.Vector2)
+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.EventHandler`1<Tizen.NUI.Scene3D.CaptureFinishedEventArgs> Tizen.NUI.Scene3D.SceneView::CaptureFinished
|
}; | ||
|
||
AsyncCaptureFinished += Handler; | ||
var captureId = Interop.SceneView.Capture(SwigCPtr, camera.SwigCPtr, Vector2.getCPtr(size)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모종의 이유로 (ex : SceneView 가 SceneOff 상태) 캡쳐가 실패한 경우 Capture API 가 끝나기 전에 CaptureFinished 콜백이 올라오도록 구현되어 있는 것이 가장 마지막 코드인데요, CaptureFinished 콜백을 Idler 에서 올려보내도록 수정 부탁드립니다.
Description of Change
This PR is to add capture to the Scene3D SceneView.
User can get a captured scene of SceneView with input Camera.
API Changes