July 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 08/10] gdi32: Where possible directly access the DC instead of using GetCurrentObject().

by Huw Davies

On Fri, Jul 29, 2016 at 09:22:43PM +0900, Alexandre Julliard wrote: > Huw Davies <huw(a)codeweavers.com> writes: > > > @@ -1039,7 +1039,7 @@ INT WINAPI SetGraphicsMode( HDC hdc, INT mode ) > > } > > release_dc_ptr( dc ); > > /* font metrics depend on the graphics mode */ > > - if (ret) SelectObject(dc->hSelf, GetCurrentObject(dc->hSelf, OBJ_FONT)); > > + if (ret) SelectObject(dc->hSelf, dc->hFont); > > The bug was already present, but we shouldn't be accessing the dc after > release. Good catch (although it was originally your bug ;-) ) I've resent the entire series with an additional patch [8/11] which fixes this. It also only reselects the font if the mode changes. The other patches in the series are essentially the same as v1. Huw.

July 29, 2016

Re: comctl32: Avoid using memset on RECTs.

by Michael Stefaniuc

On 07/29/2016 01:14 PM, Nikolay Sivov wrote: > On 29.07.2016 13:24, Michael Stefaniuc wrote: >> Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> >> --- >> dlls/comctl32/status.c | 4 ++-- >> dlls/comctl32/tab.c | 7 ++----- >> 2 files changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/dlls/comctl32/status.c b/dlls/comctl32/status.c >> index 738c33b..08ca158 100644 >> --- a/dlls/comctl32/status.c >> +++ b/dlls/comctl32/status.c >> @@ -127,8 +127,8 @@ STATUSBAR_ComputeHeight(STATUS_INFO *infoPtr) >> * textHeight pixels large */ >> HDC hdc = GetDC(infoPtr->Self); >> RECT r; >> - memset (&r, 0, sizeof (r)); >> - r.bottom = max(infoPtr->minHeight, tm.tmHeight); >> + >> + SetRect(&r, 0, 0, 0, max(infoPtr->minHeight, tm.tmHeight)); >> if (SUCCEEDED(GetThemeBackgroundExtent(theme, hdc, SP_PANE, 0, &r, &r))) >> { >> height = r.bottom - r.top; >> diff --git a/dlls/comctl32/tab.c b/dlls/comctl32/tab.c >> index 0ed9152..62e34f9 100644 >> --- a/dlls/comctl32/tab.c >> +++ b/dlls/comctl32/tab.c >> @@ -373,12 +373,9 @@ static BOOL TAB_InternalGetItemRect( >> TRACE("Not Visible\n"); >> /* need to initialize these to empty rects */ >> if (itemRect) >> - { >> - memset(itemRect,0,sizeof(RECT)); >> - itemRect->bottom = infoPtr->tabHeight; >> - } >> + SetRect(itemRect, 0, 0, 0, infoPtr->tabHeight); >> if (selectedRect) >> - memset(selectedRect,0,sizeof(RECT)); >> + SetRectEmpty(selectedRect); >> return FALSE; >> } >> >> > > You probably can remove null rect checks as well. Probably? That isn't obvious from the context. Those two are input parameters to TAB_InternalGetItemRect() that aren't checked beforehand. bye micheal

July 29, 2016

Re: [PATCH 08/10] gdi32: Where possible directly access the DC instead of using GetCurrentObject().

by Alexandre Julliard

Huw Davies <huw(a)codeweavers.com> writes: > @@ -1039,7 +1039,7 @@ INT WINAPI SetGraphicsMode( HDC hdc, INT mode ) > } > release_dc_ptr( dc ); > /* font metrics depend on the graphics mode */ > - if (ret) SelectObject(dc->hSelf, GetCurrentObject(dc->hSelf, OBJ_FONT)); > + if (ret) SelectObject(dc->hSelf, dc->hFont); The bug was already present, but we shouldn't be accessing the dc after release. -- Alexandre Julliard julliard(a)winehq.org

July 29, 2016

Re: comctl32: Avoid using memset on RECTs.

by Nikolay Sivov

On 29.07.2016 13:24, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)redhat.de> > --- > dlls/comctl32/status.c | 4 ++-- > dlls/comctl32/tab.c | 7 ++----- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/dlls/comctl32/status.c b/dlls/comctl32/status.c > index 738c33b..08ca158 100644 > --- a/dlls/comctl32/status.c > +++ b/dlls/comctl32/status.c > @@ -127,8 +127,8 @@ STATUSBAR_ComputeHeight(STATUS_INFO *infoPtr) > * textHeight pixels large */ > HDC hdc = GetDC(infoPtr->Self); > RECT r; > - memset (&r, 0, sizeof (r)); > - r.bottom = max(infoPtr->minHeight, tm.tmHeight); > + > + SetRect(&r, 0, 0, 0, max(infoPtr->minHeight, tm.tmHeight)); > if (SUCCEEDED(GetThemeBackgroundExtent(theme, hdc, SP_PANE, 0, &r, &r))) > { > height = r.bottom - r.top; > diff --git a/dlls/comctl32/tab.c b/dlls/comctl32/tab.c > index 0ed9152..62e34f9 100644 > --- a/dlls/comctl32/tab.c > +++ b/dlls/comctl32/tab.c > @@ -373,12 +373,9 @@ static BOOL TAB_InternalGetItemRect( > TRACE("Not Visible\n"); > /* need to initialize these to empty rects */ > if (itemRect) > - { > - memset(itemRect,0,sizeof(RECT)); > - itemRect->bottom = infoPtr->tabHeight; > - } > + SetRect(itemRect, 0, 0, 0, infoPtr->tabHeight); > if (selectedRect) > - memset(selectedRect,0,sizeof(RECT)); > + SetRectEmpty(selectedRect); > return FALSE; > } > > You probably can remove null rect checks as well.

July 29, 2016

Re: [PATCH] msxml3: Implement IXMLParser GetParserState

by Alistair Leslie-Hughes

On 29/07/16 20:36, Nikolay Sivov wrote: > On 29.07.2016 12:43, Alistair Leslie-Hughes wrote: > > Do you have SDK headers for that interface? Having HRESULT for a state > looks a bit weird. > > No, but all the Document on msdn state that it returns HRESULT. I had to double check myself. https://msdn.microsoft.com/en-us/library/ms757816%28v=vs.85%29.aspx Best Regards Alistair Leslie-Hughes

July 29, 2016

Re: [PATCH] msxml3: Implement IXMLParser GetParserState

by Nikolay Sivov

On 29.07.2016 12:43, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/msxml3/tests/xmlparser.c | 3 +++ > dlls/msxml3/xmlparser.c | 6 ++++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/dlls/msxml3/tests/xmlparser.c b/dlls/msxml3/tests/xmlparser.c > index 51f2a27..32514e3 100644 > --- a/dlls/msxml3/tests/xmlparser.c > +++ b/dlls/msxml3/tests/xmlparser.c > @@ -144,6 +144,9 @@ static void create_test(void) > hr = IXMLParser_SetFlags(parser, 0); > ok(hr == S_OK, "Expected S_OK got 0x%08x\n", hr); > > + hr = IXMLParser_GetParserState(parser); > + ok(hr == XMLPARSER_IDLE, "got 0x%08x\n", hr); > + > IXMLParser_Release(parser); > } > > diff --git a/dlls/msxml3/xmlparser.c b/dlls/msxml3/xmlparser.c > index ca40b22..6fcdb82 100644 > --- a/dlls/msxml3/xmlparser.c > +++ b/dlls/msxml3/xmlparser.c > @@ -51,6 +51,7 @@ typedef struct _xmlparser > LONG ref; > > int flags; > + XML_PARSER_STATE state; > } xmlparser; > > static inline xmlparser *impl_from_IXMLParser( IXMLParser *iface ) > @@ -345,9 +346,9 @@ static HRESULT WINAPI xmlparser_GetParserState(IXMLParser *iface) > { > xmlparser *This = impl_from_IXMLParser( iface ); > > - FIXME("(%p)\n", This); > + TRACE("(%p)\n", This); > > - return E_NOTIMPL; > + return This->state; > } > > static HRESULT WINAPI xmlparser_Suspend(IXMLParser *iface) > @@ -447,6 +448,7 @@ HRESULT XMLParser_create(void **ppObj) > This->nodefactory = NULL; > This->input = NULL; > This->flags = 0; > + This->state = XMLPARSER_IDLE; > This->ref = 1; > > *ppObj = &This->IXMLParser_iface; > Do you have SDK headers for that interface? Having HRESULT for a state looks a bit weird.

July 29, 2016

Re: [PATCH 2/6] d3d9/tests: Add visual tests for locking vertexbuffers

by Henri Verbeet

On 28 July 2016 at 18:41, Patrick Rudolph <siro(a)das-labor.org> wrote: > Am Thu, 28 Jul 2016 14:04:56 +0200 > schrieb Henri Verbeet <hverbeet(a)gmail.com>: >> I assume the broken() is for the testbot, but the way this is written, >> half the cases aren't really tested on Windows. Also, what makes these >> fail on the testbot? >> > I can replace some of the broken test results with another flag for AMD > behaviour. AMD allows D3DLOCK_NOOVERWRITE for non dynamic buffers on > d3d9, too. > I guess the WARP driver filters D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD > calls. You can make the check more specific by using adapter_is_warp(). Wrt. D3DLOCK_NOOVERWRITE on non-dynamic buffers, if the behaviour is not consistent between AMD and NVIDIA, it's probably best to drop those tests while keeping the comment. >> Does this work reliably? GetTickCount() has very limited resolution, I >> think there's a real chance that the draw will take only a fraction of >> GetTickCount()'s resolution. > > Yes it does. > I first thought about using timestamp queries, but those aren't > supported on ddraw and d3d8 and are broken on my AMD test system, so > I'm using GetTickCount instead. It allows to test to pass on fast > systems (Intel Core i5 + HD6870) and slow systems (Intel Atom + Nvidia > ION). The buffersize can't be hardcoded as the Windows driver nukes the > display driver whenever a draw call takes more than a few seconds, > which is the case when using huge buffers to make them pass on fast > systems. > Measuring CPU time is fine, but I'd feel a little better if this used QueryPerformanceCounter() instead of GetTickCount().

July 29, 2016

Re: wined3d: Add Nvidia GT 740M to the GPU list

by Henri Verbeet

On 28 July 2016 at 22:42, Jeremy Audiger <audiger(a)live.fr> wrote: > @@ -2158,6 +2160,7 @@ cards_nvidia_mesa[] = > {"NVF0", CARD_NVIDIA_GEFORCE_GTX780}, > {"NVE6", CARD_NVIDIA_GEFORCE_GTX770M}, > {"NVE4", CARD_NVIDIA_GEFORCE_GTX680}, > + {"NV108", CARD_NVIDIA_GEFORCE_GT740M}, > /* Fermi */ > {"NVD9", CARD_NVIDIA_GEFORCE_GT520}, > {"NVD7", CARD_NVIDIA_GEFORCE_820M}, The patch seems fine, but could you please add "NV108" above "NVF1" instead of below "NVE4"?

July 29, 2016

Re: [PATCH 2/2] DInput : Use vendor and product ids to generate product GUID like it's done on Windows

by DavidL

Thanks! In that case, I will add it to my list of needed patches for the OS X dinput joysticks ... unless one of you gents would like to take a crack at it? :) Cheers, David On Thu, Jul 28, 2016 at 8:44 AM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > On Thu, Jul 28, 2016 at 6:08 AM, DavidL <david.dljunk(a)gmail.com> wrote: > > I'm not really familiar with the intricacies of the guid generation ... > but > > looking at the Mac dinput code ... should a similar patch be applied > there? > > Hi, IMO yes in order to sync the output of all drivers to be the same > independent of host OS. You can test using the ditool.exe from: > https://sourceforge.net/projects/x360ce/files/ditool.zip > > Best wishes, > Bruno >

July 28, 2016

Re: [PATCH 1/6] wined3d/resource: Add flag WINED3D_MAP_WRITEONLY

by Patrick Rudolph

Am Thu, 28 Jul 2016 14:04:48 +0200 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > On 26 July 2016 at 19:09, Patrick Rudolph <siro(a)das-labor.org> wrote: > > #define WINED3D_MAP_READONLY > > 0x0010 +#define > > WINED3D_MAP_WRITEONLY 0x0020 > But now the API is weird. What happens when both READONLY and > WRITEONLY are set? I think the d3d11 model makes more sense. Yes you are right. The patch is required to fix some of the d3d11 TODOs introduced in my tests, but you can ignore it for now. I'll add additional tests for it.

July 28, 2016