Re: [2/4] wined3d: Track when the FBO is dirty.
On 03/27/2011 06:45 AM, Henri Verbeet wrote:
On 21 March 2011 21:57, Adam Martinson wrote:
@@ -2102,6 +2121,7 @@ BOOL context_apply_clear_state(struct wined3d_context
*context, IWineD3DDeviceIm
if (rt_count< context->gl_info->limits.buffers)
memset(context->blit_targets + rt_count, 0,
(context->gl_info->limits.buffers - rt_count) * sizeof(*rts));
context_apply_fbo_state(context, GL_FRAMEBUFFER, rt_count,
context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+context->device_fbo = context->current_fbo;
You can't do that.
@@ -4801,6 +4801,7 @@ static HRESULT WINAPI
IWineD3DDeviceImpl_Clear(IWineD3DDevice *iface, DWORD rect
}
device_get_draw_rect(device,&draw_rect);
+IWineD3DDeviceImpl_MarkFBODirty(device);
What is this for?
+static inline void IWineD3DDeviceImpl_MarkFBODirty(IWineD3DDeviceImpl *This)
This doesn't need to be in wined3d_private.h, it's only used from device.c.
+int i;
+for (i = 0; i< This->numContexts; ++i)
numContexts is unsigned.
What would happen when the current draw fbo entry would get evicted
from the cache? Also, while the patch avoids redundant lookups for
draws, it doesn't do a lot for clears. Is that intentional because
it's not worth it, or is there a different reason for that?
I think I've addressed all of these... attached try #2.
>From d0ead7dd57d17d7ea0a590e66bea4e42cba873da Mon Sep 17 00:00:00 2001
From: Adam Martinson
Date: Wed, 6 Apr 2011 23:28:25 -0500
Subject: [PATCH 2/3] wined3d: Track when the FBO is dirty.
To: wine-patches
Reply-To: wine-devel
---
dlls/wined3d/context.c | 49 +++-
dlls/wined3d/device.c | 16 +
dlls/wined3d/wined3d_private.h |1 +
3 files changed, 65 insertions(+), 1 deletions(-)
diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c
index 5b5ae5a..a074fa7 100644
--- a/dlls/wined3d/context.c
+++ b/dlls/wined3d/context.c
@@ -374,6 +374,9 @@ static void context_reuse_fbo_entry(struct wined3d_context *context, GLenum targ
entry->depth_stencil = depth_stencil;
entry->location = location;
entry->attached = FALSE;
+
+if (entry == context->device_fbo)
+context->device_fbo = NULL;
}
/* GL locking is done by the caller */
@@ -473,6 +476,24 @@ static void context_apply_fbo_entry(struct wined3d_context *context, GLenum targ
}
/* GL locking is done by the caller */
+static void context_apply_device_fbo(struct wined3d_context *context)
+{
+struct fbo_entry *entry = context->device_fbo;
+if (context->rebind_fbo)
+{
+context_bind_fbo(context, GL_FRAMEBUFFER, NULL);
+context->rebind_fbo = FALSE;
+}
+
+list_remove(&entry->entry);
+list_add_head(&context->fbo_list, &entry->entry);
+
+context->current_fbo = entry;
+context_apply_fbo_entry(context, GL_FRAMEBUFFER, entry);
+context_check_fbo_status(context, GL_FRAMEBUFFER);
+}
+
+/* GL locking is done by the caller */
static void context_apply_fbo_state(struct wined3d_context *context, GLenum target,
IWineD3DSurfaceImpl **render_targets, IWineD3DSurfaceImpl *depth_stencil, DWORD location)
{
@@ -1406,6 +1427,7 @@ struct wined3d_context *context_create(IWineD3DSwapChainImpl *swapchain,
ret->render_offscreen = surface_is_offscreen(target);
ret->draw_buffer_dirty = TRUE;
+ret->device_fbo = NULL;
ret->valid = 1;
ret->glCtx = ctx;
@@ -2097,7 +2119,25 @@ BOOL context_apply_clear_state(struct wined3d_context *context, IWineD3DDeviceIm
context->blit_targets[i] = NULL;
++i;
}
-context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+
+if (rt_count == device->adapter->gl_info.limits.buffers &&
+rts == device->render_targets &&
+depth_stencil == device->depth_stencil)
+{
+if (context->device_fbo != NULL)
+{
+context_apply_device_fbo(context);
+}
+else
+{
+context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+context->device_fbo = context->current_fbo;
+}
+}
+else
+{
+context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+}
}
else
{
@@ -2160,12 +2200,19 @@ BOOL context_apply_draw_state(struct wined3d_context *context, IWineD3DDeviceImp
context_apply_fbo_state(context, GL_FRAMEBUFFER, NULL, NULL, SFLAG_INDRAWABLE);
LEAVE_GL();
}
+else if (context->device_fbo != NULL)
+{
+ENTER_GL();
+context_apply_device_fbo(context);
+LEAVE_GL();
+
Re: [2/4] wined3d: Track when the FBO is dirty.
On 04/07/2011 10:54 AM, Henri Verbeet wrote:
On 7 April 2011 17:40, Adam Martinson wrote:
I think I've addressed all of these... attached try #2.
It doesn't compile.
Heh... someone changed a variable name on me, sorry.
>From d0ead7dd57d17d7ea0a590e66bea4e42cba873da Mon Sep 17 00:00:00 2001
From: Adam Martinson
Date: Wed, 6 Apr 2011 23:28:25 -0500
Subject: [PATCH 2/3] wined3d: Track when the FBO is dirty.
To: wine-patches
Reply-To: wine-devel
---
dlls/wined3d/context.c | 49 +++-
dlls/wined3d/device.c | 16 +
dlls/wined3d/wined3d_private.h |1 +
3 files changed, 65 insertions(+), 1 deletions(-)
diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c
index 5b5ae5a..a074fa7 100644
--- a/dlls/wined3d/context.c
+++ b/dlls/wined3d/context.c
@@ -374,6 +374,9 @@ static void context_reuse_fbo_entry(struct wined3d_context *context, GLenum targ
entry->depth_stencil = depth_stencil;
entry->location = location;
entry->attached = FALSE;
+
+if (entry == context->device_fbo)
+context->device_fbo = NULL;
}
/* GL locking is done by the caller */
@@ -473,6 +476,24 @@ static void context_apply_fbo_entry(struct wined3d_context *context, GLenum targ
}
/* GL locking is done by the caller */
+static void context_apply_device_fbo(struct wined3d_context *context)
+{
+struct fbo_entry *entry = context->device_fbo;
+if (context->rebind_fbo)
+{
+context_bind_fbo(context, GL_FRAMEBUFFER, NULL);
+context->rebind_fbo = FALSE;
+}
+
+list_remove(&entry->entry);
+list_add_head(&context->fbo_list, &entry->entry);
+
+context->current_fbo = entry;
+context_apply_fbo_entry(context, GL_FRAMEBUFFER, entry);
+context_check_fbo_status(context, GL_FRAMEBUFFER);
+}
+
+/* GL locking is done by the caller */
static void context_apply_fbo_state(struct wined3d_context *context, GLenum target,
IWineD3DSurfaceImpl **render_targets, IWineD3DSurfaceImpl *depth_stencil, DWORD location)
{
@@ -1406,6 +1427,7 @@ struct wined3d_context *context_create(IWineD3DSwapChainImpl *swapchain,
ret->render_offscreen = surface_is_offscreen(target);
ret->draw_buffer_dirty = TRUE;
+ret->device_fbo = NULL;
ret->valid = 1;
ret->glCtx = ctx;
@@ -2097,7 +2119,25 @@ BOOL context_apply_clear_state(struct wined3d_context *context, IWineD3DDeviceIm
context->blit_targets[i] = NULL;
++i;
}
-context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+
+if (rt_count == device->adapter->gl_info.limits.buffers &&
+rts == device->render_targets &&
+depth_stencil == device->depth_stencil)
+{
+if (context->device_fbo != NULL)
+{
+context_apply_device_fbo(context);
+}
+else
+{
+context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+context->device_fbo = context->current_fbo;
+}
+}
+else
+{
+context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, depth_stencil, SFLAG_INTEXTURE);
+}
}
else
{
@@ -2160,12 +2200,19 @@ BOOL context_apply_draw_state(struct wined3d_context *context, IWineD3DDeviceImp
context_apply_fbo_state(context, GL_FRAMEBUFFER, NULL, NULL, SFLAG_INDRAWABLE);
LEAVE_GL();
}
+else if (context->device_fbo != NULL)
+{
+ENTER_GL();
+context_apply_device_fbo(context);
+LEAVE_GL();
+}
else
{
ENTER_GL();
context_apply_fbo_state(context, GL_FRAMEBUFFER, device->render_targets,
device->depth_stencil, SFLAG_INTEXTURE);
LEAVE_GL();
+context->device_fbo = context->current_fbo;
}
}
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index e91a078..11a395e 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -453,6 +453,15 @@ void device_update_stream_info(IWineD3DDeviceImpl *device, const struct wined3d_
}
}
+static inline void IWineD3DDeviceImpl_MarkFBODirty(IWineD3DDeviceImpl *This)
+{
+UINT i;
+for (i = 0; i < This->context_count; ++i)
+{
+This->contexts[i]->device_fbo = NULL;
+}
+}
+
static void device_preload_texture(const struct wined3d_state *state, unsigned int idx)
{
struct wined3d_texture *texture;
@@ -5866,6 +5875,7 @@ static HRESULT WINAPI IWineD3DDeviceImpl_SetRenderTarget(IWineD3DDevice *iface,
if (render_target) IWineD3DSurface_AddRef(render_target);
device->render_targets[render_target_idx] = (IWineD3DSurfaceImpl *)render_target;
+
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On 8 April 2011 08:09, Charles Davis wrote: > > I don't know if this will help but... > > Both Clang and (recent) GCC have direct support for > __attribute__((thiscall)) (and I would know about Clang, I added it to > the LLVM side). We could potentially take advantage of this and not have > to declare thunks like this. That's definitely not portable. It could be used with ifdefs, but then one would have to ask if it is worth the effort. Is there any speed or security or other reason for preferring this method on compilers that support it?
Re: Google Summer of Code project
Hi Michał, On 4/8/11 12:28 AM, Michał Ziętek wrote: Hello, My name is Michal Zietek, I am studying Computer Science at Wroclaw University of Technology. I would like to take part at Google Summer of Code. Lately I saw Jacek Caban presenting Wine at the university and he said there could be a possibility to start in Google Summer of Code and work on Wine. I talked to Jacek about project related to WScript. I am interested in working on this project but I can't find it on the Google Summer of Code website. The only place I found any information is http://www.winehq.org/pipermail/wine-devel/2011-February/088839.html Do you consider putting this project into Google Summer of Code? Sure, the project is fine, I just forgot to put it on wiki. Feel free to send an application, but note that the deadline is tomorrow. Jacek
Re: Google Summer of Code project
On 4/7/11 4:28 PM, Michał Ziętek wrote: > Do you consider putting this project into Google Summer of Code? You can propose whatever you want for GSoC (as long as it's relevant to Wine, or whatever organization you're proposing to). That page is just a list of ideas to get you started. In fact, I proposed something myself you won't find on that list: a graphics driver that uses the native graphics system on Mac OS X. Chip
Re: Google Summer of Code project
On Thu, Apr 7, 2011 at 15:28, Michał Ziętek wrote: > Hello, > > My name is Michal Zietek, I am studying Computer Science at Wroclaw > University of Technology. I would like to take part at Google Summer of > Code. Lately I saw Jacek Caban presenting Wine at the university and he said > there could be a possibility to start in Google Summer of Code and work on > Wine. I talked to Jacek about project related to WScript. I am interested in > working on this project but I can't find it on the Google Summer of Code > website. The only place I found any information is > http://www.winehq.org/pipermail/wine-devel/2011-February/088839.html > > Do you consider putting this project into Google Summer of Code? Under Google Summer of Code, you have to apply for your own project. In other words, you tell Wine what you would like to do to make Wine better. The application for applications is tomorrow: http://www.google-melange.com/document/show/gsoc_program/google/gsoc2011/faqs#student http://www.google-melange.com/gsoc/events/google/gsoc2011 There is some more information here: http://wiki.winehq.org/SummerOfCode Good luck! -- -Austin
Google Summer of Code project
Hello, My name is Michal Zietek, I am studying Computer Science at Wroclaw University of Technology. I would like to take part at Google Summer of Code. Lately I saw Jacek Caban presenting Wine at the university and he said there could be a possibility to start in Google Summer of Code and work on Wine. I talked to Jacek about project related to WScript. I am interested in working on this project but I can't find it on the Google Summer of Code website. The only place I found any information is http://www.winehq.org/pipermail/wine-devel/2011-February/088839.html Do you consider putting this project into Google Summer of Code? Best regards, Michal Zietek.
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On 4/7/11 3:28 PM, Michael Stefaniuc wrote: > On 04/07/2011 07:04 PM, Dylan Smith wrote: >> On Thu, Apr 7, 2011 at 1:00 PM, Dylan Smith >> wrote: >>> >>> The rest of the richedit code needs to call the ITextHost interface >>> using the thiscall calling convention, so on i386 it calls a thunk in >>> itextHostStdcallVtbl which are defined using the stdcall calling >>> convention, and perform stdcall->thiscall conversion before jumping to >>> the thiscall defined method (i.e. the actual method for user provided >>> ITextHost, and a thunk to reverse the calling convention in Wine). >> >> Sorry, that last part should read (i.e. the actual method for user >> provided ITextHost, OR a thunk to reverse the calling convention for >> calling the internal ITextHostImpl). > Thanks for the explanation. It kinda makes sense but it still feels ugly. I don't know if this will help but... Both Clang and (recent) GCC have direct support for __attribute__((thiscall)) (and I would know about Clang, I added it to the LLVM side). We could potentially take advantage of this and not have to declare thunks like this. Chip
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On 04/07/2011 07:04 PM, Dylan Smith wrote: On Thu, Apr 7, 2011 at 1:00 PM, Dylan Smith wrote: The rest of the richedit code needs to call the ITextHost interface using the thiscall calling convention, so on i386 it calls a thunk in itextHostStdcallVtbl which are defined using the stdcall calling convention, and perform stdcall->thiscall conversion before jumping to the thiscall defined method (i.e. the actual method for user provided ITextHost, and a thunk to reverse the calling convention in Wine). Sorry, that last part should read (i.e. the actual method for user provided ITextHost, OR a thunk to reverse the calling convention for calling the internal ITextHostImpl). Thanks for the explanation. It kinda makes sense but it still feels ugly. bye michael
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On Thu, Apr 7, 2011 at 1:00 PM, Dylan Smith wrote: > > The rest of the richedit code needs to call the ITextHost interface > using the thiscall calling convention, so on i386 it calls a thunk in > itextHostStdcallVtbl which are defined using the stdcall calling > convention, and perform stdcall->thiscall conversion before jumping to > the thiscall defined method (i.e. the actual method for user provided > ITextHost, and a thunk to reverse the calling convention in Wine). Sorry, that last part should read (i.e. the actual method for user provided ITextHost, OR a thunk to reverse the calling convention for calling the internal ITextHostImpl).
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On Thu, Apr 7, 2011 at 12:17 PM, Michael Stefaniuc wrote: > > Dylan Smith wrote: > > On Thu, Apr 7, 2011 at 5:30 AM, Michael Stefaniuc wrote: > > > >> The COM methods are already __stdcall. > >> --- > >> dlls/riched20/txthost.c | 128 > >> ++ > >> 1 files changed, 39 insertions(+), 89 deletions(-) > >> > >> COM methods are supposed to use the stdcall calling convention, but the > > native riched20 and the native headers just defines ITextHost and > > ITextServices to be a c++ class (i.e. using the thiscall calling > > convention). Just look at the native TextServ.h header. > Right, but for that you have the textHostVtbl with the THISCALL() wrapper. > > The itextHostStdcallVtbl is just for the C COM macros used from Wine > code as calling a thiscall function from Wine is a PITA. That is handled > in editor.h: > #ifdef __i386__ /* Use wrappers to perform thiscall on i386 */ > #define TXTHOST_VTABLE(This) (&itextHostStdcallVtbl) > #else /* __i386__ */ > #define TXTHOST_VTABLE(This) (This)->lpVtbl > #endif /* __i386__ */ > /*** ITextHost methods ***/ > #define ITextHost_TxGetDC(This) TXTHOST_VTABLE(This)->TxGetDC(This) > #define ITextHost_TxReleaseDC(This,a) > TXTHOST_VTABLE(This)->TxReleaseDC(This,a) > ... > > On i386 Wine is calling functions from the static itextHostStdcallVtbl > and those functions are not thiscall. Of course I might miss something > as that code is mind warping... > I'll try to explain. The ITextHost interface may be user provided for windowless richedit controls, but for windowed richedit controls the internal ITextHostImpl implementation is used. They both must be defined using the thiscall calling convention, so in wine ITextHostImpl has a set of thunks to fill the textHostVtbl that perform thiscall->stdcall conversion before jumping to the stdcall defined methods. The rest of the richedit code needs to call the ITextHost interface using the thiscall calling convention, so on i386 it calls a thunk in itextHostStdcallVtbl which are defined using the stdcall calling convention, and perform stdcall->thiscall conversion before jumping to the thiscall defined method (i.e. the actual method for user provided ITextHost, and a thunk to reverse the calling convention in Wine).
Re: [2/4] wined3d: Track when the FBO is dirty.
On 7 April 2011 18:55, Adam Martinson wrote: > Heh... someone changed a variable name on me, sorry. > Well yeah, things change. But how did you run the tests if the patch didn't compile?
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
Dylan Smith wrote: > On Thu, Apr 7, 2011 at 5:30 AM, Michael Stefaniuc wrote: > >> The COM methods are already __stdcall. >> --- >> dlls/riched20/txthost.c | 128 >> ++ >> 1 files changed, 39 insertions(+), 89 deletions(-) >> >> COM methods are supposed to use the stdcall calling convention, but the > native riched20 and the native headers just defines ITextHost and > ITextServices to be a c++ class (i.e. using the thiscall calling > convention). Just look at the native TextServ.h header. Right, but for that you have the textHostVtbl with the THISCALL() wrapper. The itextHostStdcallVtbl is just for the C COM macros used from Wine code as calling a thiscall function from Wine is a PITA. That is handled in editor.h: #ifdef __i386__ /* Use wrappers to perform thiscall on i386 */ #define TXTHOST_VTABLE(This) (&itextHostStdcallVtbl) #else /* __i386__ */ #define TXTHOST_VTABLE(This) (This)->lpVtbl #endif /* __i386__ */ /*** ITextHost methods ***/ #define ITextHost_TxGetDC(This) TXTHOST_VTABLE(This)->TxGetDC(This) #define ITextHost_TxReleaseDC(This,a) TXTHOST_VTABLE(This)->TxReleaseDC(This,a) ... On i386 Wine is calling functions from the static itextHostStdcallVtbl and those functions are not thiscall. Of course I might miss something as that code is mind warping... bye michael
Re: [2/4] wined3d: Track when the FBO is dirty.
On 7 April 2011 17:40, Adam Martinson wrote: > I think I've addressed all of these... attached try #2. > It doesn't compile.
Re: riched20: Remove the unneeded DEFINE_STDCALL_WRAPPER.
On Thu, Apr 7, 2011 at 5:30 AM, Michael Stefaniuc wrote: > The COM methods are already __stdcall. > --- > dlls/riched20/txthost.c | 128 > ++ > 1 files changed, 39 insertions(+), 89 deletions(-) > > COM methods are supposed to use the stdcall calling convention, but the native riched20 and the native headers just defines ITextHost and ITextServices to be a c++ class (i.e. using the thiscall calling convention). Just look at the native TextServ.h header.
__CxxFrameHandler unsupported on wine64?
I have an application that requires MFC80.DLL. When I install only the MFC80.DLL file from the vcredist package, I end up with the following errors: wine: Call from 0x7fd606a3148b to unimplemented function MSVCR80.dll.__CxxFrameHandler, aborting So, I had a quick look at the wine source and found a comment in dlls/msvcrt/cppexcept.c that says: /* CxxFrameHandler is not supported on non-i386 */ I don't think that this is quite right, given that Microsoft's MFC80.DLL attempts to call __CxxFrameHandler. Is it more likely that wine only provides an i386 implementation? If so, is there any chance that someone may be able to provide x86_64 implementation in the near future? It's way over my head :-( Best regards, Peter Urbanec
Re: [PATCH 2/6] winmm: DriverCallback ignores a 0 CALLBACK_WINDOW.
writes: > - I thought about merging patches 2+3+5 into one or two. After all, > they all change the one tiny DriverCallback function. > E.g. patch 2 fixes the corner case where Wine delivers a spurious > message to the thread message queue, while patch 5 generalizes > the behaviour that MS mentions (and I tested), i.e. ignore any 0 dwCallback. > OTOH, split is split. They have to be merged somehow, it doesn't make sense to add code that gets removed again in the next patch. -- Alexandre Julliard julli...@winehq.org
[PATCH 2/6] winmm: DriverCallback ignores a 0 CALLBACK_WINDOW.
Hi, any feedback on winmm:DriverCallback patches 2-6? - Although I superseded patch 1/6, the others are unchanged, hence I did not resend them. - I thought about merging patches 2+3+5 into one or two. After all, they all change the one tiny DriverCallback function. E.g. patch 2 fixes the corner case where Wine delivers a spurious message to the thread message queue, while patch 5 generalizes the behaviour that MS mentions (and I tested), i.e. ignore any 0 dwCallback. OTOH, split is split. - It may seem weird to go to some effort to make DriverCallback return the exact same values as native *and* then change all drivers to ignore that value, so the patches may be considered superfluous. Yet we strive to be bug-compatible. Regards, Jörg Höhle
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
Piotr Caban writes: > It happens when native _resetstkoflw is called when there's little > memory left on the stack. I don't know if there's any real application > that is calling it in that case. I thought that it's incorrect to grow > the stack size above stack limit anyway. No, the stack limit indicates the limit of the current growth. If the stack grows the limit is updated. -- Alexandre Julliard julli...@winehq.org
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
On 04/07/11 11:22, Alexandre Julliard wrote: Piotr Caban writes: Without this patch there's following condition for stack growing: if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) NtCurrentTeb()->Tib.StackLimit = page; If after growing the stack application protects the memory between StackLimit and StackLimit-page_size, StackLimit will be changed again. StackLimit is supposed to be the last address that is unprotected, so changing it in that case would be correct. The last page should really to remain protected though, which application is modifying it? It happens when native _resetstkoflw is called when there's little memory left on the stack. I don't know if there's any real application that is calling it in that case. I thought that it's incorrect to grow the stack size above stack limit anyway.
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
Piotr Caban writes: > Without this patch there's following condition for stack growing: > if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) > NtCurrentTeb()->Tib.StackLimit = page; > If after growing the stack application protects the memory between > StackLimit and StackLimit-page_size, StackLimit will be changed again. StackLimit is supposed to be the last address that is unprotected, so changing it in that case would be correct. The last page should really to remain protected though, which application is modifying it? -- Alexandre Julliard julli...@winehq.org
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
On 04/07/11 11:00, Alexandre Julliard wrote: Piotr Caban writes: It's not needed (I thought it may be not valid for whole stack). I'll send fixed version. There also should be "growing" instead of "shrinking" in commit message. It was meant to point that it's possible to change StackLimit more then once without this patch. I'm not sure what you mean, making it grow is the reason for that function. Without this patch there's following condition for stack growing: if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit) NtCurrentTeb()->Tib.StackLimit = page; If after growing the stack application protects the memory between StackLimit and StackLimit-page_size, StackLimit will be changed again.
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
Piotr Caban writes: > It's not needed (I thought it may be not valid for whole stack). I'll > send fixed version. > > There also should be "growing" instead of "shrinking" in commit > message. It was meant to point that it's possible to change StackLimit > more then once without this patch. I'm not sure what you mean, making it grow is the reason for that function. -- Alexandre Julliard julli...@winehq.org
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
On 04/07/11 10:24, Alexandre Julliard wrote:
Piotr Caban writes:
@@ -1617,9 +1617,18 @@ BOOL virtual_handle_stack_fault( void *addr )
BYTE vprot = view->prot[((const char *)page - (const char
*)view->base)>> page_shift];
if (vprot& VPROT_GUARD)
{
+struct _TEB *teb = NtCurrentTeb();
VIRTUAL_SetProt( view, page, page_size, vprot& ~VPROT_GUARD );
-if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit)
-NtCurrentTeb()->Tib.StackLimit = page;
+if ((char *)page - page_size == teb->DeallocationStack)
+teb->Tib.StackLimit = page;
+else if ((char*)addr> (char*)teb->Tib.StackLimit + page_size&&
+teb->Tib.StackLimit == (char*)teb->DeallocationStack +
page_size&&
+(view = VIRTUAL_FindView( (char*)teb->Tib.StackLimit, 0)))
+{
+vprot = view->prot[((char*)teb->Tib.StackLimit -
(char*)view->base)>> page_shift];
+VIRTUAL_SetProt( view, teb->Tib.StackLimit, page_size, vprot |
VPROT_GUARD);
+teb->Tib.StackLimit = (char*)teb->Tib.StackLimit + page_size;
+}
Why do you want to lookup the view again?
It's not needed (I thought it may be not valid for whole stack). I'll
send fixed version.
There also should be "growing" instead of "shrinking" in commit message.
It was meant to point that it's possible to change StackLimit more then
once without this patch.
Re: gsoc theming
Thank you so much for your assistance and patience. The thing that confuses me is you want only one dll for comctl32 Though the only way i can think of to do this is to to have an xml parser inside the dll and then trying to load the correct manifest file. To check if the manifest exists or asks for version 6+. There may be some api for this. Though I havn't been able to find it. At this link in the Window Class section I assume is what you referred to about the comctl32 having a manifest in our previous discussions. So i'm trying to get my head around that. http://msdn.microsoft.com/en-us/library/aa374219(v=vs.85).aspx On Mon, Apr 4, 2011 at 11:19 PM, Nikolay Sivov wrote: > On 4/4/2011 16:45, Reece Dunn wrote: >> >> On 4 April 2011 13:08, Andrew Green wrote: >>> >>> Hi, >>> I hate to message you on this email. I have messaged the mailing >>> lists. Though I became more confused than anything. >> >> Can you please keep the discussions on the mailing list? Thanks. >> >>> I would like to propose a project where comctl32 is implement >>> correctly in windows for google summer of code. >>> As far as i know windows uses SxS to load version 6 of comctl allowing >>> theming. >> >> This is correct -- an application needs to add a manifest file to tell >> it to load version 6 of the comctl32 DLL in order to get theming >> support on Windows. >> >>> It also uses windows class redirection to hijack controls >>> from user32 to be themed(I don't know if it somehow uses subclassing). >> >> This is how it is implemented in wine. > > Native doesn't subclass, it has its own copy of so called builtin controls > in comctl32. > First evidence for that is a special class registration call from comctl32 > v6 to register themed classes. > Second evidence is that new Button functionality for example is available > only in comctl32 v6, not in latest user32 module, > I doubt they customize existing behaviour, I think it's more likely some > message handlers are reused (compiled in both modules) and some are present > in comctl32 only. > > > >>> The confusing thing on the mailing list i was told not to make a >>> separate version of comctl32. Which is understandable from a code >>> maintenance point of view. Though contradicts what the gsoc ideas page >>> says >>> "control is overridden by a themed drawing version by subclassing. >>> This is not the way it is implemented on Windows and this causes >>> drawing issues and other nasty issues." >>> As someone who is to be a mentor of this section. What is the goal? >>> and if subclassing is the option, how is this meant to work? >> >> There isn't a contradition. >> >> AFAICS, the subclassing works -- the subclassed window procedure gets >> called correctly. >> >> The drawing issues can be seen clearly on the button controls -- if >> the button changes state (enabled<-> disabled, pressed, etc.) then it >> gets drawn without theming. This is because the button control in Wine >> does not call WM_PAINT to do the drawing, which the themed button >> overrides. > > Subclass may work fine of course, but could cause problems. For example it's > possible that application clears > all subclass slots (if we're talking about comctl32 mechanism with procedure > stack). When you create button with v6 on windows you don't get sublcassed > control (stack is empty), that may be important. > > Anyway if you subclass let's say a Button you need to override lot of > handlers potentially, cause for example new window styles could be rejected > by modern user32 but accepted in comctl32 logic. That could cause a mess > when different code works with not necessary valid control data. This needs > some testing. > > Another question is to determine if we need to use controls from comctl32 in > a first place. If you have active theme it doesn't mean that developer > wanted to use it if he doesn't specify manifest to load version 6. > > So to summarize, the way I see this to be properly done step by step: > > 1) add class redirect detection for activation context code > (ntdll/kernel32). Returned structures are undocumented, but if you dump data > you'll see something (I did); > 2) (hard part, for me at least) figure out when to invoke class registration > call from comctl32 to move registration to comctl32. > 3) add tests for user32 to see how builtin classes respond to new version 6 > styles/messages; > 4) figure out what code could be reused between user32 and comctl32 and > reuse as much handlers from user32 as possible. This will need some build > tweaks probably, I'm not sure, to share files between dlls. > > What also bothers me is that I'm not sure comctl32 versions of controls (in > windows) use A/W magic for procedures that user32 uses. >> >> Therefore, you will need to either: >> 1/ convince Alexandre through tests that these tests do call >> WM_PAINT [*] and update the non-themed control accordingly; or >> 2/ implement the state tracking/logic in the themed button and have >> it call W
Re: [PATCH 1/2] ntdll: Restore stack guard and prevent stack from shrinking
Piotr Caban writes:
> @@ -1617,9 +1617,18 @@ BOOL virtual_handle_stack_fault( void *addr )
> BYTE vprot = view->prot[((const char *)page - (const char
> *)view->base) >> page_shift];
> if (vprot & VPROT_GUARD)
> {
> +struct _TEB *teb = NtCurrentTeb();
> VIRTUAL_SetProt( view, page, page_size, vprot & ~VPROT_GUARD );
> -if ((char *)page + page_size == NtCurrentTeb()->Tib.StackLimit)
> -NtCurrentTeb()->Tib.StackLimit = page;
> +if ((char *)page - page_size == teb->DeallocationStack)
> +teb->Tib.StackLimit = page;
> +else if ((char*)addr > (char*)teb->Tib.StackLimit + page_size &&
> +teb->Tib.StackLimit == (char*)teb->DeallocationStack +
> page_size &&
> +(view = VIRTUAL_FindView( (char*)teb->Tib.StackLimit,
> 0)))
> +{
> +vprot = view->prot[((char*)teb->Tib.StackLimit -
> (char*)view->base) >> page_shift];
> +VIRTUAL_SetProt( view, teb->Tib.StackLimit, page_size, vprot
> | VPROT_GUARD);
> +teb->Tib.StackLimit = (char*)teb->Tib.StackLimit + page_size;
> +}
Why do you want to lookup the view again?
--
Alexandre Julliard
julli...@winehq.org
