[vlmc-devel] [PATCH 4/5] RenderTarget: Widget

[Hugo Beauzée-Luyssen]

Hi,

I'm not sure what you mean by «We should use a number to store either a 
pointer or a number because
deleting m_id as a void pointer may cause a problem. Instead we should
delete the value of m_id.»
Why would you delete m_id?
We might need to think of a way to clearly express that the 
Backend::WidgetTarget must not outlive the RendererWidget it's using, 
but I don't think the Backend::WidgetTarget should delete anything.

On 04/14/2016 10:55 AM, Yikai Lu wrote:
> ---
>   src/Backend/Target/WidgetTarget.cpp | 17 +++++++++++++++++
>   src/Backend/Target/WidgetTarget.h   | 18 ++++++++++++++++++
>   src/CMakeLists.txt                  |  1 +
>   3 files changed, 36 insertions(+)
>   create mode 100644 src/Backend/Target/WidgetTarget.cpp
>   create mode 100644 src/Backend/Target/WidgetTarget.h
>
> diff --git a/src/Backend/Target/WidgetTarget.cpp b/src/Backend/Target/WidgetTarget.cpp
> new file mode 100644
> index 0000000..f6d33c7
> --- /dev/null
> +++ b/src/Backend/Target/WidgetTarget.cpp
> @@ -0,0 +1,17 @@
> +#include "WidgetTarget.h"
> +
> +#include "Backend/ISourceRenderer.h"
> +
> +using namespace Backend;
> +
> +WidgetTarget::WidgetTarget( void* id )
> +    : m_id( reinterpret_cast<long long int>( id ) )

This looks like problems on 32bits platforms. Please use (u)intptr_t 
instead it you want to store a pointer as an integer.
It could be a better idea to have a conditional type, such as 
QWidget::WId, that holds the proper type, depending on the OS

> +{
> +}
> +
> +void
> +WidgetTarget::configure( ISourceRenderer *renderer )
> +{
> +    renderer->setOutputWidget( reinterpret_cast<void*>( m_id ) );
> +}
> +
> diff --git a/src/Backend/Target/WidgetTarget.h b/src/Backend/Target/WidgetTarget.h
> new file mode 100644
> index 0000000..e22d39f
> --- /dev/null
> +++ b/src/Backend/Target/WidgetTarget.h
> @@ -0,0 +1,18 @@
> +#ifndef WIDGETTARGET_H
> +#define WIDGETTARGET_H
> +
> +#include "Backend/IRenderTarget.h"
> +
> +namespace Backend
> +{
> +    class WidgetTarget : public IRenderTarget
> +    {
> +    public:
> +        WidgetTarget( void* id );
> +        virtual void configure( ISourceRenderer *renderer );
> +    private:
> +        long long int m_id;
> +    };
> +}
> +
> +#endif // WIDGETTARGET_H
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index ae426ff..4c59e63 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -46,6 +46,7 @@ SET(VLMC_SRCS
>       Backend/VLC/VLCVmemRenderer.cpp
>       Backend/VLC/VLCSource.cpp
>       Backend/VLC/VLCMemorySource.cpp
> +    Backend/Target/WidgetTarget.cpp
>       EffectsEngine/EffectsEngine.cpp
>       EffectsEngine/Effect.cpp
>       EffectsEngine/EffectUser.cpp
>