[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH 2/2] d3drm: Store animation options
From:       Nikolay Sivov <bunglehead () gmail ! com>
Date:       2017-06-27 17:35:28
Message-ID: 22272c3e-364f-2405-e201-538efd6f2926 () gmail ! com
[Download RAW message or body]

On 27.06.2017 17:43, Henri Verbeet wrote:
> On 27 June 2017 at 16:33, Nikolay Sivov <bunglehead@gmail.com> wrote:
> > On 27.06.2017 15:43, Henri Verbeet wrote:
> > > On 27 June 2017 at 10:17, Nikolay Sivov <nsivov@codeweavers.com> wrote:
> > > > +static HRESULT WINAPI d3drm_animation2_SetOptions(IDirect3DRMAnimation2 \
> > > > *iface, D3DRMANIMATIONOPTIONS options) +{
> > > > +    struct d3drm_animation *animation = \
> > > > impl_from_IDirect3DRMAnimation2(iface); +    static const DWORD \
> > > > supported_options = D3DRMANIMATION_OPEN | D3DRMANIMATION_CLOSED | \
> > > > D3DRMANIMATION_LINEARPOSITION +        | D3DRMANIMATION_SPLINEPOSITION | \
> > > > D3DRMANIMATION_SCALEANDROTATION | D3DRMANIMATION_POSITION; +
> > > > +    TRACE("iface %p, options %#x.\n", iface, options);
> > > > +
> > > > +    if (options && !(options & supported_options))
> > > > +        return D3DRMERR_BADVALUE;
> > > Should that be "if (options & ~supported_options)"? The test is
> > > ambiguous about that.
> > 
> > No, they both are wrong. It should be just "if (!(options &
> > supported_options)", because 0 is not allowed.
> > 
> What about unrecognised options? E.g., would 0x80000001 be allowed or not?

Yes, it is allowed, and stored as is - unsupported bits are not filtered
out. So patch v2 works correctly in that regard, only additional test is
missing.

> 
> > Bottom line is spline and linear mode bits are bad design,
> > 
> Yeah, but that goes for much of ddraw/d3d up to about version 9 or 10.
> 


[prev in list] [next in list] [prev in thread] [next in thread]