[vlmc-devel] [PATCH 5/7] GenericRenderer Implement setRenderTarget

[yikei lu]

I'll use std::move :)

2016-04-17 17:42 GMT+09:00 yikei lu <luyikei.qmltu at gmail.com>:
> oh well we might want to use unique_ptr internally in GenericRenderer
>
>
> 2016-04-17 17:35 GMT+09:00 yikei lu <luyikei.qmltu at gmail.com>:
>> well then the ownership of the pointer will change. And if
>> GenericRenderer should have the ownership, I think we don't have to
>> use unique_ptr.
>>
>> Or we can use std::unique_ptr outside GenericRenderer, and pass it as
>> a raw pointer, which won't change the ownership
>>
>>
>> 2016-04-17 17:29 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
>>> On 04/17/2016 10:27 AM, yikei lu wrote:
>>>>
>>>> I think it would be better to use shared_ptr rather than unique_ptr
>>>> because we we could call setRenderTarget outside the
>>>> GenericRenderer-based classes,
>>>>
>>>>
>>>>
>>>> 2016-04-17 17:00 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
>>>>>
>>>>> On 04/17/2016 09:45 AM, yikei lu wrote:
>>>>>>
>>>>>>
>>>>>> Yeah, I also once thought that it would be great to use unique_ptr,
>>>>>> but still I don't really know when we should use it since I don't use
>>>>>> it so much. :)
>>>>>>
>>>>>> I think this is simple enough to manage it, but it might be better to
>>>>>> use unique_pt because when we setRenderTarget, the pointer should be
>>>>>> used only in GenericRenderer. But the parent renderer has the
>>>>>> ownership of the pointer.
>>>>>>
>>>>>> Yes I think it's safe to use unique_pt :)
>>>>>> Regards
>>>>>>
>>>>>> 2016-04-17 16:34 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
>>>>>>>
>>>>>>>
>>>>>>> On 04/16/2016 08:22 AM, Yikai Lu wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>     src/Renderer/GenericRenderer.cpp | 8 ++++++++
>>>>>>>>     src/Renderer/GenericRenderer.h   | 5 +++++
>>>>>>>>     2 files changed, 13 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/src/Renderer/GenericRenderer.cpp
>>>>>>>> b/src/Renderer/GenericRenderer.cpp
>>>>>>>> index 598b59e..49ea01b 100644
>>>>>>>> --- a/src/Renderer/GenericRenderer.cpp
>>>>>>>> +++ b/src/Renderer/GenericRenderer.cpp
>>>>>>>> @@ -27,6 +27,7 @@
>>>>>>>>     GenericRenderer::GenericRenderer()
>>>>>>>>         : m_sourceRenderer( nullptr )
>>>>>>>>         , m_paused( false )
>>>>>>>> +    , m_renderTarget( nullptr )
>>>>>>>>     {
>>>>>>>>         m_eventWatcher = new RendererEventWatcher;
>>>>>>>>     }
>>>>>>>> @@ -35,6 +36,7 @@ GenericRenderer::~GenericRenderer()
>>>>>>>>     {
>>>>>>>>         delete m_sourceRenderer;
>>>>>>>>         delete m_eventWatcher;
>>>>>>>> +    delete m_renderTarget;
>>>>>>>>     }
>>>>>>>>
>>>>>>>>     bool
>>>>>>>> @@ -49,6 +51,12 @@ GenericRenderer::isRendering() const
>>>>>>>>         return m_isRendering;
>>>>>>>>     }
>>>>>>>>
>>>>>>>> +void
>>>>>>>> +GenericRenderer::setRenderTarget( Backend::IRenderTarget *target )
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I'm enclined to think this should take a std::unique_ptr instead of a
>>>>>>> raw
>>>>>>> one, opinion?
>>>>>>>
>>>>>>>
>>>>>>>> +{
>>>>>>>> +    m_renderTarget = target;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>     #ifdef WITH_GUI
>>>>>>>>     RendererEventWatcher*
>>>>>>>>     GenericRenderer::eventWatcher()
>>>>>>>> diff --git a/src/Renderer/GenericRenderer.h
>>>>>>>> b/src/Renderer/GenericRenderer.h
>>>>>>>> index 6fb326a..fbc9a5e 100644
>>>>>>>> --- a/src/Renderer/GenericRenderer.h
>>>>>>>> +++ b/src/Renderer/GenericRenderer.h
>>>>>>>> @@ -29,6 +29,7 @@
>>>>>>>>
>>>>>>>>     #include "Workflow/Types.h"
>>>>>>>>     #include "Tools/RendererEventWatcher.h"
>>>>>>>> +#include "Backend/IRenderTarget.h"
>>>>>>>>
>>>>>>>>     class   Clip;
>>>>>>>>     class   Media;
>>>>>>>> @@ -135,6 +136,8 @@ public:
>>>>>>>>          */
>>>>>>>>         bool                            isRendering() const;
>>>>>>>>
>>>>>>>> +    void                            setRenderTarget(
>>>>>>>> Backend::IRenderTarget* target );
>>>>>>>> +
>>>>>>>>         RendererEventWatcher*           eventWatcher();
>>>>>>>>     protected:
>>>>>>>>         Backend::ISourceRenderer*       m_sourceRenderer;
>>>>>>>> @@ -150,6 +153,8 @@ protected:
>>>>>>>>         bool                            m_paused;
>>>>>>>>         bool                            m_isRendering;
>>>>>>>>
>>>>>>>> +    Backend::IRenderTarget*         m_renderTarget;
>>>>>>>> +
>>>>>>>>     public slots:
>>>>>>>>         /**
>>>>>>>>          *  \brief      This SLOT will be called when the time cursor
>>>>>>>> has
>>>>>>>> changed.
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Vlmc-devel mailing list
>>>>>>> Vlmc-devel at videolan.org
>>>>>>> https://mailman.videolan.org/listinfo/vlmc-devel
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Vlmc-devel mailing list
>>>>>> Vlmc-devel at videolan.org
>>>>>> https://mailman.videolan.org/listinfo/vlmc-devel
>>>>>>
>>>>> Ok, I'll wait for your updated patch then :)
>>>>> Thanks!
>>>>>
>>>>> _______________________________________________
>>>>> Vlmc-devel mailing list
>>>>> Vlmc-devel at videolan.org
>>>>> https://mailman.videolan.org/listinfo/vlmc-devel
>>>>
>>>> _______________________________________________
>>>> Vlmc-devel mailing list
>>>> Vlmc-devel at videolan.org
>>>> https://mailman.videolan.org/listinfo/vlmc-devel
>>>>
>>> I don't see why that would change with unique_ptr ?
>>>
>>> _______________________________________________
>>> Vlmc-devel mailing list
>>> Vlmc-devel at videolan.org
>>> https://mailman.videolan.org/listinfo/vlmc-devel