Re: [PATCH 2/2] dwrite: Store text format properties

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> +static HRESULT WINAPI dwritetextformat_SetLineSpacing(IDWriteTextFormat *iface, DWRITE_LINE_SPACING_METHOD method,
> - FLOAT spacing, FLOAT baseline)
>
> {
>      struct dwrite_textformat *This = impl_from_IDWriteTextFormat(iface);
> - FIXME("(%p)->(%d %f %f): stub\n", This, spacing, line_spacing, baseline);
> - return E_NOTIMPL;
> - TRACE("(%p)->(%d %f %f)\n", This, method, spacing, baseline);
> - This->format.spacingmethod = method;
> - This->format.spacing = spacing;
> - This->format.baseline = baseline;
> - return S_OK;
>
> }

Probably all the format properties setters should handle invalid enum or
negative floating point arguments some way.

[PATCH 2/2] dwrite: Store text format properties

[Nikolay Sivov]

On 4/7/2014 09:29, Dmitry Timoshkov wrote:
> Nikolay Sivov  wrote:
> > +static HRESULT WINAPI dwritetextformat_SetLineSpacing(IDWriteTextFormat *iface, DWRITE_LINE_SPACING_METHOD method,
> > - FLOAT spacing, FLOAT baseline)
> > {
> >  struct dwrite_textformat *This = impl_from_IDWriteTextFormat(iface);
> > - FIXME("(%p)->(%d %f %f): stub\n", This, spacing, line_spacing, baseline);
> > - return E_NOTIMPL;
> > - TRACE("(%p)->(%d %f %f)\n", This, method, spacing, baseline);
> > - This->format.spacingmethod = method;
> > - This->format.spacing = spacing;
> > - This->format.baseline = baseline;
> > - return S_OK;
> > }
>
> Probably all the format properties setters should handle invalid enum or
> negative floating point arguments some way.

It's too early to care about that at this point.

[PATCH 2/2] dwrite: Store text format properties

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> > > +static HRESULT WINAPI dwritetextformat_SetLineSpacing(IDWriteTextFormat *iface, DWRITE_LINE_SPACING_METHOD method,
> > > - FLOAT spacing, FLOAT baseline)
> > > {
> > >  struct dwrite_textformat *This = impl_from_IDWriteTextFormat(iface);
> > > - FIXME("(%p)->(%d %f %f): stub\n", This, spacing, line_spacing, baseline);
> > > - return E_NOTIMPL;
> > > - TRACE("(%p)->(%d %f %f)\n", This, method, spacing, baseline);
> > > - This->format.spacingmethod = method;
> > > - This->format.spacing = spacing;
> > > - This->format.baseline = baseline;
> > > - return S_OK;
> > > }
> >
> > Probably all the format properties setters should handle invalid enum or
> > negative floating point arguments some way.
>
> It's too early to care about that at this point.

I'm afraid that it will be too late when some application sets invalid value
and that would lead to weird rendering results for unknown reason.

[PATCH 2/2] dwrite: Store text format properties

[Nikolay Sivov]

On 4/7/2014 10:39, Dmitry Timoshkov wrote:
> Nikolay Sivov  wrote:
> > > > +static HRESULT WINAPI dwritetextformat_SetLineSpacing(IDWriteTextFormat *iface, DWRITE_LINE_SPACING_METHOD method,
> > > > - FLOAT spacing, FLOAT baseline)
> > > > {
> > > >   struct dwrite_textformat *This = impl_from_IDWriteTextFormat(iface);
> > > > - FIXME("(%p)->(%d %f %f): stub\n", This, spacing, line_spacing, baseline);
> > > > - return E_NOTIMPL;
> > > > - TRACE("(%p)->(%d %f %f)\n", This, method, spacing, baseline);
> > > > - This->format.spacingmethod = method;
> > > > - This->format.spacing = spacing;
> > > > - This->format.baseline = baseline;
> > > > - return S_OK;
> > > > }
> > >
> > > Probably all the format properties setters should handle invalid enum or
> > > negative floating point arguments some way.
> >
> > It's too early to care about that at this point.
>
> I'm afraid that it will be too late when some application sets invalid value
> and that would lead to weird rendering results for unknown reason.

For now I only need it to succeed, once it starts to draw something this 
will be revised or will be redone if needed.