Re: [v3 PATCH] gdi32: Support Begin/End Path for metafile DCs
On Tue, Feb 16, 2016 at 09:28:06PM +1100, Alistair Leslie-Hughes wrote:
From: Dmitry Timoshkov <dmitry(a)baikal.ru>
Correct failed test. Updated Author/signoffs. v3 - Removed Dmitry signoff on request.
Fixes https://bugs.winehq.org/show_bug.cgi?id=39185
Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/gdi32/enhmfdrv/dc.c | 4 +++- dlls/gdi32/path.c | 20 +++++++++++++++++++- dlls/gdi32/tests/metafile.c | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/dlls/gdi32/path.c b/dlls/gdi32/path.c index e09cd0b..166b6b2 100644 --- a/dlls/gdi32/path.c +++ b/dlls/gdi32/path.c ... @@ -1513,6 +1523,14 @@ static BOOL pathdrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const REC struct path_physdev *physdev = get_path_physdev( dev ); unsigned int idx, ggo_flags = GGO_NATIVE; POINT offset = {0, 0}; + DWORD type; + + type = GetObjectType(dev->hdc); + if (type == OBJ_METADC || type == OBJ_ENHMETADC) + { + PHYSDEV next = GET_NEXT_PHYSDEV( dev, pExtTextOut ); + return next->funcs->pExtTextOut( next, x, y, flags, lprc, str, count, dx ); + }
if (!count) return TRUE; if (flags & ETO_GLYPH_INDEX) ggo_flags |= GGO_GLYPH_INDEX;
Why the special treatment for ExtTextOut()? I can believe we may need to do something with Begin/EndPath, but why this? Dmitry, is this the reason you don't want to send it? Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
diff --git a/dlls/gdi32/path.c b/dlls/gdi32/path.c index e09cd0b..166b6b2 100644 --- a/dlls/gdi32/path.c +++ b/dlls/gdi32/path.c ... @@ -1513,6 +1523,14 @@ static BOOL pathdrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const REC struct path_physdev *physdev = get_path_physdev( dev ); unsigned int idx, ggo_flags = GGO_NATIVE; POINT offset = {0, 0}; + DWORD type; + + type = GetObjectType(dev->hdc); + if (type == OBJ_METADC || type == OBJ_ENHMETADC) + { + PHYSDEV next = GET_NEXT_PHYSDEV( dev, pExtTextOut ); + return next->funcs->pExtTextOut( next, x, y, flags, lprc, str, count, dx ); + }
if (!count) return TRUE; if (flags & ETO_GLYPH_INDEX) ggo_flags |= GGO_GLYPH_INDEX;
Why the special treatment for ExtTextOut()? I can believe we may need to do something with Begin/EndPath, but why this?
According to the tests a path on metafiles just creates a EMR_EXTTEXTOUT record instead of generating a bunch of actual path representation records.
Dmitry, is this the reason you don't want to send it?
No. -- Dmitry.
On Tue, Feb 16, 2016 at 07:38:33PM +0800, Dmitry Timoshkov wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
diff --git a/dlls/gdi32/path.c b/dlls/gdi32/path.c index e09cd0b..166b6b2 100644 --- a/dlls/gdi32/path.c +++ b/dlls/gdi32/path.c ... @@ -1513,6 +1523,14 @@ static BOOL pathdrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const REC struct path_physdev *physdev = get_path_physdev( dev ); unsigned int idx, ggo_flags = GGO_NATIVE; POINT offset = {0, 0}; + DWORD type; + + type = GetObjectType(dev->hdc); + if (type == OBJ_METADC || type == OBJ_ENHMETADC) + { + PHYSDEV next = GET_NEXT_PHYSDEV( dev, pExtTextOut ); + return next->funcs->pExtTextOut( next, x, y, flags, lprc, str, count, dx ); + }
if (!count) return TRUE; if (flags & ETO_GLYPH_INDEX) ggo_flags |= GGO_GLYPH_INDEX;
Why the special treatment for ExtTextOut()? I can believe we may need to do something with Begin/EndPath, but why this?
According to the tests a path on metafiles just creates a EMR_EXTTEXTOUT record instead of generating a bunch of actual path representation records.
But isn't that just because we only test ExtTextOut()? For example, we don't even test the contents of the emf created in test_emf_GetPath(). What I'm wondering is whether we need to do this to more than just ExtTextOut.
Dmitry, is this the reason you don't want to send it?
No.
That doesn't exactly inspire confidence in this patch... Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
diff --git a/dlls/gdi32/path.c b/dlls/gdi32/path.c index e09cd0b..166b6b2 100644 --- a/dlls/gdi32/path.c +++ b/dlls/gdi32/path.c ... @@ -1513,6 +1523,14 @@ static BOOL pathdrv_ExtTextOut( PHYSDEV dev, INT x, INT y, UINT flags, const REC struct path_physdev *physdev = get_path_physdev( dev ); unsigned int idx, ggo_flags = GGO_NATIVE; POINT offset = {0, 0}; + DWORD type; + + type = GetObjectType(dev->hdc); + if (type == OBJ_METADC || type == OBJ_ENHMETADC) + { + PHYSDEV next = GET_NEXT_PHYSDEV( dev, pExtTextOut ); + return next->funcs->pExtTextOut( next, x, y, flags, lprc, str, count, dx ); + }
if (!count) return TRUE; if (flags & ETO_GLYPH_INDEX) ggo_flags |= GGO_GLYPH_INDEX;
Why the special treatment for ExtTextOut()? I can believe we may need to do something with Begin/EndPath, but why this?
According to the tests a path on metafiles just creates a EMR_EXTTEXTOUT record instead of generating a bunch of actual path representation records.
But isn't that just because we only test ExtTextOut()? For example, we don't even test the contents of the emf created in test_emf_GetPath().
That's a good point. EMF contents get tested in test_emf_ExtTextOut_on_path() and test_mf_ExtTextOut_on_path() though. Alistair, please add a test for EMF contents in addition to what GetPath() returns, see other tests how to do that, there is a helper that will generate appropriate data for you.
What I'm wondering is whether we need to do this to more than just ExtTextOut.
That's also a good point, but it would be a separate effort I'd guess. -- Dmitry.
On Tue, Feb 16, 2016 at 08:04:00PM +0800, Dmitry Timoshkov wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
But isn't that just because we only test ExtTextOut()? For example, we don't even test the contents of the emf created in test_emf_GetPath().
That's a good point. EMF contents get tested in test_emf_ExtTextOut_on_path() and test_mf_ExtTextOut_on_path() though. Alistair, please add a test for EMF contents in addition to what GetPath() returns, see other tests how to do that, there is a helper that will generate appropriate data for you.
Adding something more than MoveTo/LineTo would be good too. In addition a test for wmfs would be interesting.
What I'm wondering is whether we need to do this to more than just ExtTextOut.
That's also a good point, but it would be a separate effort I'd guess.
If we need to do this for many functions, we probably want to look for another way; so I'd like to understand the problem first. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
What I'm wondering is whether we need to do this to more than just ExtTextOut.
That's also a good point, but it would be a separate effort I'd guess.
If we need to do this for many functions, we probably want to look for another way; so I'd like to understand the problem first.
I skimmed over the EMR_ list in wingdi.h together with pathdrv_ implemented entries and gathered a list of possible candidates: EMR_ELLIPSE EMR_PIE EMR_POLYBEZIER EMR_POLYBEZIERTO EMR_POLYDRAW EMR_POLYPOLYGON EMR_POLYLINE EMR_POLYLINETO EMR_RECTANGLE EMR_ROUNDRECT Would you mind listing the EMRs that you think also may need testing? -- Dmitry.
On Tue, Feb 16, 2016 at 08:42:39PM +0800, Dmitry Timoshkov wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
What I'm wondering is whether we need to do this to more than just ExtTextOut.
That's also a good point, but it would be a separate effort I'd guess.
If we need to do this for many functions, we probably want to look for another way; so I'd like to understand the problem first.
I skimmed over the EMR_ list in wingdi.h together with pathdrv_ implemented entries and gathered a list of possible candidates: EMR_ELLIPSE EMR_PIE EMR_POLYBEZIER EMR_POLYBEZIERTO EMR_POLYDRAW EMR_POLYPOLYGON EMR_POLYLINE EMR_POLYLINETO EMR_RECTANGLE EMR_ROUNDRECT
Would you mind listing the EMRs that you think also may need testing?
That looks good to me. Frankly, we probably don't need to test them all, just a reasonable subset should give us an idea of how to proceed. Thanks, Huw.
participants (2)
-
Dmitry Timoshkov -
Huw Davies