Re: GetKeyboardState problems in Age of Empires II
On Thu, 18 Apr 2013 17:39:33 +0900 Dmitry Timoshkov dmi...@baikal.ru wrote: [please don't omit wine-devel when replying] Stevie Trujillo stevie.truji...@gmail.com wrote: Stevie Trujillo stevie.truji...@gmail.com wrote: Another (minor) problem raised in the bugzilla entry is that, if a key is pressed when changing window, and released before returning, GetKeyboardState() will claim the key is still pressed (0x80). But this is easy to workaround, just hit the key again inside the game and it will stop scrolling. dlls/winex11.drv/keyboard.c,X11DRV_KeymapNotify() should take care of that, if it doesn't - please debug why (probably a bug in your WM). I think this function only applies to modifier keys. It was tested with arrow keys. It should work for all keys. Anyway I don't think it's a big issue, since it doesn't cause any big problem, and it doesn't happen very often. It was just a situation where Windows and Wine exhibited slightly different behavior. The first bug is much more annoying since one has to completely exit the game each time. My comment was targeting both problems that you mentioned. Ok, I didn't understand what this function does with the arrow keys yet, but I think I found out where the 0x40 comes from now: In server/queue.c::update_input_key_state, down = (keystate == desktop-keystate) ? 0xc0 : 0x80; When server/queue.c::set_input_key_state() is called with down=0, 0xc0 ~0x80 = 0x40, keystate[key] = ~0x80; From dlls/winex11.drv/keyboard.c::X11DRV_KeymapNotify, get_async_key_state does SERVER_START_REQ( get_key_state ) with req-tid = 0. This retrieves the desktop-keystate. When writing back changes, set_async_key_state calls SERVER_START_REQ( set_key_state ) with req-tid = GetCurrentThreadId(). This should copy the 0x40 bits from the desktop's keystate to the thread's keystate.
Re: GetKeyboardState problems in Age of Empires II
I tried to make a test case now. It's tested on wine-1.5.27 since I
don't have enough battery to compile new wine.
$ winegcc test.c -o test
Issue1, the 0x40 bit:
$ ./test.exe
00 00 00 00
00 00 00 00
*press LEFT arrow*
81 00 00 00
*release LEFT arrow*
01 00 00 00
*ALTTAB*
41 00 00 00
Issue2, LEFT ARROW remains pressed if released in another window.
$ ./test.exe
00 00 00 00
*press LEFT ARROW*
81 00 00 00
*ALTTAB*
c1 00 00 00
*RELEASE LEFT ARROW (outside Wine application)*
c1 00 00 00
*ALTTAB BACK*
c1 00 00 00
*press LEFT ARROW again (inside Wine application)*
41 00 00 00
test.c (also at http://pastie.org/7642788 if my mail client breaks it):
// based on http://www.winprog.org/tutorial/simple_window.html
#include windows.h
#include stdio.h
static const char g_szClassName[] = myWindowClass;
static void print_keyboard_state()
{
BYTE keystate[256];
GetKeyboardState(keystate);
printf(%02x %02x %02x %02x\n, keystate[VK_LEFT], keystate[VK_UP],
keystate[VK_RIGHT], keystate[VK_DOWN]);
}
// Step 4: the Window Procedure
static LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam,
LPARAM lParam)
{
switch(msg)
{
case WM_TIMER:
print_keyboard_state();
break;
case WM_CLOSE:
DestroyWindow(hwnd);
break;
case WM_DESTROY:
PostQuitMessage(0);
break;
default:
return DefWindowProc(hwnd, msg, wParam, lParam);
}
return 0;
}
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
LPSTR lpCmdLine, int nCmdShow)
{
WNDCLASSEX wc;
HWND hwnd;
MSG Msg;
//Step 1: Registering the Window Class
wc.cbSize= sizeof(WNDCLASSEX);
wc.style = 0;
wc.lpfnWndProc = WndProc;
wc.cbClsExtra= 0;
wc.cbWndExtra= 0;
wc.hInstance = hInstance;
wc.hIcon = LoadIcon(NULL, IDI_APPLICATION);
wc.hCursor = LoadCursor(NULL, IDC_ARROW);
wc.hbrBackground = (HBRUSH)(COLOR_WINDOW+1);
wc.lpszMenuName = NULL;
wc.lpszClassName = g_szClassName;
wc.hIconSm = LoadIcon(NULL, IDI_APPLICATION);
if(!RegisterClassEx(wc))
return 1;
// Step 2: Creating the Window
hwnd = CreateWindowEx(
WS_EX_CLIENTEDGE,
g_szClassName,
The title of my window,
WS_OVERLAPPEDWINDOW,
CW_USEDEFAULT, CW_USEDEFAULT, 240, 120,
NULL, NULL, hInstance, NULL);
if(hwnd == NULL)
return 1;
ShowWindow(hwnd, nCmdShow);
UpdateWindow(hwnd);
UINT id;
SetTimer(hwnd, 0, 1000, NULL);
// Step 3: The Message Loop
while(GetMessage(Msg, NULL, 0, 0) 0)
{
TranslateMessage(Msg);
DispatchMessage(Msg);
}
return Msg.wParam;
}
Re: [PATCH 2/2] gdi32: Clip font glyphs to fit within text metrics.
Sam Edwards cfswo...@gmail.com wrote: This change only affects malformed fonts that have glyphs with splines that go above the maximum ascent as specified in the font's hhea/os2 table. For that reason, any tests for this change would have to include a malformed font, so I did not write tests. If we want in-tree tests, we'll need to figure out how to include a malformed font to test against. Please add a test case, Wine test suite already includes custom fonts, you can either use an existing font, or add a new one. -- Dmitry.
Re: Clang static analyzer results / wine-1.5.28-66-g6899279
On 04/19/2013 12:43 AM, Austin English wrote: With wine-1.5.20 and clang 3.2, the test suite is in the same state on my Fedora 18 machine as gcc-4.7.2 llvm version: git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@179768 91177308-0d34-0410-b5e6-96231b3b80d8 clang version: git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179771 91177308-0d34-0410-b5e6-96231b3b80d8 ./configure --disable-tests was used to reduce noise Static analyse of the tests is important as well. Over the years I found bad tests that didn't test what they were supposed to test. And even cases where the Wine code was bad due to that as it passed the tests. bye michael
Google Summer of Code 2013
Sir, I will be applying for GSoC 2013 under your mentoring team. Sorry for contacting very late with you as I was not aware of this program. I came to know yesterday that Gsoc is going to start. As a CS student and I have a well understanding of C C++ and Java and Data structures I am always very keenly interested in programming and coding. I devote my daily 8 hrs to it. As a potential linux user I have chosen Ideas from your list Cygwin Improvement I wish to apply on this topic and devote my each and every day of this summer to help my friends and colleagues to run Wine without bugs. Well one more thing to say Sir please give more Information and details what I have to do to assure my project to be done. Regards Anulesh Tiwari Bachelor of Science Department of Computer Science BHU INDIA
Re: [PATCH 1/3] msvcrt: Added _mbsncoll and _mbsncoll_l implementation.
On Fri, 19 Apr 2013 14:42:50 +0800, Jactry Zeng wrote:
+/*
+ * _mbsncoll_l(MSVCRT.@)
+ */
+int CDECL _mbsncoll_l(const unsigned char *str1, const unsigned char *str2,
MSVCRT_size_t count,
+ MSVCRT__locale_t locale)
+{
+return _mbsnbcoll_l(str1, str2, count, locale);
+}
According to MSDN, _mbsncoll_l receives a number of characters, but
_mbsnbcoll_l receives a number of bytes. This is a big difference in
multibyte code page.
So you should properly handle this situation, or add FIXME message based
on multibyte code page if your application just needs this function.
Regards,
Akihiro Sagawa
Re: [PATCH 1/3] msvcrt: Added _mbsncoll and _mbsncoll_l implementation.
Hi Akihiro, 2013/4/19 Akihiro Sagawa sagawa@gmail.com According to MSDN, _mbsncoll_l receives a number of characters, but _mbsnbcoll_l receives a number of bytes. This is a big difference in multibyte code page. So you should properly handle this situation, or add FIXME message based on multibyte code page if your application just needs this function. Thanks for your review. :-) I will tried to improve it. -- Regards, Jactry Zeng
Re: [PATCH 2/7] wined3d: Store vs_clipping in the adapter
On 19 April 2013 11:20, Stefan Dösinger ste...@codeweavers.com wrote: Potential alternatives include keeping the status quo or introducing a separate structure that holds these derived limits that is owned by the device and linked in the contexts. We could also put it into gl_info, but I dislike this idea because these limits are not GL limits, but limits of our shader and fixed function pipeline implementations. I think we'll want a separate structure for this. Note that you essentially have two different kinds of data you're concerned with here though. One is derived state like e.g. the texture unit map, the stream info, or the view_indent from patch 1/7. We don't really want that in wined3d_state, because there's no reason for e.g. every stateblock to have a copy of that. Having the derived state structure be owned by the device is probably ok, although there may also be something to be said for making it part of the context instead. The other type of data is immutable data like derived caps and limits, as in this patch and following ones. In principle the adapter is the right place for that, but I do think we want a separate structure for that, and probably a pointer to it in the context, for similar reasons as having one to gl_info. Arguably things like the state table, format info and the shader backend would belong in the same structure. I.e., essentially we'd have a setup like wined3d_gl_info, but for more wined3d specific / derived info.
Re: [PATCH 2/7] wined3d: Use ARB_internalformat_query2 to check for texture format rendering and blending support, where available.
On 18 April 2013 21:19, Matteo Bruni mbr...@codeweavers.com wrote:
+if (!format-glInternal) continue;
+
+gl_info-gl_ops.ext.p_glGetInternalformativ(GL_TEXTURE_2D,
format-glInternal,
+GL_FRAMEBUFFER_RENDERABLE, 1, value);
+if (value == GL_FULL_SUPPORT)
+{
+TRACE(Format %s is supported as FBO color attachment.\n,
debug_d3dformat(format-id));
+format-flags |= WINED3DFMT_FLAG_RENDERTARGET |
WINED3DFMT_FLAG_FBO_ATTACHABLE;
+format-rtInternal = format-glInternal;
+
+if (!(format-flags (WINED3DFMT_FLAG_DEPTH |
WINED3DFMT_FLAG_STENCIL)))
+query_format_flag(gl_info, format, format-glInternal,
GL_FRAMEBUFFER_BLEND,
+WINED3DFMT_FLAG_POSTPIXELSHADER_BLENDING,
post-pixelshader blending);
+}
This may be a bit dangerous, since you're now potentially adding
WINED3DFMT_FLAG_RENDERTARGET for formats like e.g.
WINED3DFMT_R8G8B8A8_SNORM that didn't previously had it, and where
it's a bit questionable if it's really going to work as intended.
Also, the depth/stencil check that follows is either redundant, or
we're now also potentially setting WINED3DFMT_FLAG_RENDERTARGET for
depth/stencil formats. At least initially we'll probably want to stick
with formats that already have WINED3DFMT_FLAG_RENDERTARGET set, and
only clear it if unsupported. At a later point we may want to do
something more centered around the information we get from
internalformat_query2, but I suspect it will be a bit harder than just
checking if there's no fixup, no conversion function, etc.
Re: [PATCH 3/7] wined3d: Use ARB_internalformat_query2 for the other texture format caps too.
On 18 April 2013 21:20, Matteo Bruni mbr...@codeweavers.com wrote: +/* FIXME: We can't handle partial sRGB decode support right now, we could + * introduce a new format flag and check for that. */ As mentioned on IRC, I don't think this is really an issue. The patch looks otherwise ok.
Re: [PATCH 2/7] wined3d: Store vs_clipping in the adapter
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 2013-04-19 16:26, schrieb Henri Verbeet: I think we'll want a separate structure for this. Note that you essentially have two different kinds of data you're concerned with here though. One is derived state like e.g. the texture unit map, the stream info, or the view_indent from patch 1/7. We don't really want that in wined3d_state, because there's no reason for e.g. every stateblock to have a copy of that. Having the derived state structure be owned by the device is probably ok, although there may also be something to be said for making it part of the context instead. My plan is to move the tex unit map and stream info into the context. This is fairly easy to do because it's already the context that triggers the update of these data structures. The tricky bit is the lowest_disabled_stage because we have two pieces of code that use it. One is wined3d_device_set_texture_stage_state, which uses it to find nop state changes. The other one are the state application function and device_map_fixed_function_samplers. The state seems like the best place for this, as both the device and command stream have a copy of the state where they can keep their version of this value up to date. IMHO it makes sense to keep view_ident close to the view matrix, as there's a pretty direct relationship between them. The other place where it could go is /dev/null, I doubt that it has a noticeable performance impact. This needs some benchmarking though. load_base_vertex_index can probably go into the context. At least I don't see a reason that speaks against this. There's a related situation I'm not quite happy with: The dirty states are stored in the context, but the values that should be set are not. For now I've kept this setup in my command stream patches and didn't run into problems. There may be a nicer approach though. The other type of data is immutable data like derived caps and limits, as in this patch and following ones. In principle the adapter is the right place for that, but I do think we want a separate structure for that, and probably a pointer to it in the context, for similar reasons as having one to gl_info. Works for me. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRcWVLAAoJEN0/YqbEcdMwDAAP/21t5qSPPuX+CR5ur3qj9qe2 OfSJQUfmw0L5PMETxzq8TMcGI4qjcI11GxvocL9JBsft7aYxLWKJibwOcGvcdRJO iaoHjYow2xfRMP7QgTrlP+Ii/tfpHHJHW4kw54pcKVEJnFbiDtMBGvi/IdFnXiW3 vSJAlaPojtLz2rbWSlImCiT8b3DgQ43KY4YCNSGvsG/ATXrhOpTAPPDEZ2h/28Xp oKQ8zunob8YVWmb4zl7BJjRWJy3cj21gUgnPxLdmpqsbZn1Y6tPQj5iGOdfkb0sN yieR1y25JNs5L6C+PrbIUW22lrrAzmvFm62f2/kugwWROdZK0gSAD2L0rdfuo5CN xE76iZqdCoDyMy/PyEgPtMzKIwCHa7mOtfHCkpHvHHnBdLEcMTdXA7pFvKmiiZnB bAseA+06jMl3+qphGfk4W381LCE3OUKE5t05KD1zqNhM26muh3+TC4GO702FPq+N heIFuw3exC9f4EEyfbDJ21PtLVDFQqemWRdyfTy8UmjgFrEB9vr1HRdPnLUj4YZV V7s/bh4/Wzco29N//1oiQ5K2CoqX+AtbnMDtPoFd5/ox8BMbJF+H4JojeX14NH+B 183dWJUC6fMve5trVenw5dkq56tv53Tls58NepH6k51TVFh7KRn5tkVDRjlfywUp gvBE82NsnSWxDZi6rnMG =WAaZ -END PGP SIGNATURE-
Re: [PATCH 2/7] wined3d: Store vs_clipping in the adapter
On 19 April 2013 17:39, Stefan Dösinger stefandoesin...@gmail.com wrote: The tricky bit is the lowest_disabled_stage because we have two pieces of code that use it. One is wined3d_device_set_texture_stage_state, which uses it to find nop state changes. The other one are the state application function and device_map_fixed_function_samplers. The state seems like the best place for this, as both the device and command stream have a copy of the state where they can keep their version of this value up to date. It may be ok to just keep that in wined3d_state. I'm not sure wined3d_device_set_texture_stage_state() would really need to touch it though. While you could in theory avoid submitting state changes above lowest_disabled_stage through the CS, I'm not sure it would really gain you anything, and it could very well be more complicated than just submitting those, and only using lowest_disabled_stage on the CS side to avoid dirtifying states.
Re: [PATCH 2/2] gdi32: Clip font glyphs to fit within text metrics.
On 04/19/2013 02:46 AM, Dmitry Timoshkov wrote: Sam Edwards cfswo...@gmail.com wrote: This change only affects malformed fonts that have glyphs with splines that go above the maximum ascent as specified in the font's hhea/os2 table. For that reason, any tests for this change would have to include a malformed font, so I did not write tests. If we want in-tree tests, we'll need to figure out how to include a malformed font to test against. Please add a test case, Wine test suite already includes custom fonts, you can either use an existing font, or add a new one. As I understand it, some fonts deliberately have glyphs larger than their metrics bounding boxes. Clipping them is almost certainly not a good idea.
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Hi Dan, Thanks for working on it! It is really an annoying bug, however, is adding DECLSPEC_NOINLINE a workaround or a real fix? If it is only a workaround, would that hide some real bug behind it? Or is that possible that there is a gcc bug here that we want to report to gcc? Thanks for your time!
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
I suspect this is a real fix, and there is no gcc bug. On Fri, Apr 19, 2013 at 9:47 AM, Qian Hong fract...@gmail.com wrote: Hi Dan, Thanks for working on it! It is really an annoying bug, however, is adding DECLSPEC_NOINLINE a workaround or a real fix? If it is only a workaround, would that hide some real bug behind it? Or is that possible that there is a gcc bug here that we want to report to gcc? Thanks for your time!
Re: [PATCH 2/7] wined3d: Use ARB_internalformat_query2 to check for texture format rendering and blending support, where available.
Hrm, I sent the wrong version of the patch, I had actually moved the depth/stencil check earlier (so yeah, totally agree on that part). Also you're right about WINED3DFMT_FLAG_RENDERTARGET, for some reason I thought we were setting that in check_fbo_compat() but it's clearly not the case. I'm sending the updated patches in a bit (also dropping patches 5-7 since they are superseded by yours).
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Sat, Apr 20, 2013 at 12:52 AM, Dan Kegel d...@kegel.com wrote: I suspect this is a real fix, and there is no gcc bug. Thanks for clarify. Curiosity killed the cat, what is the theory behind this patch? I tried explicitly add 'inline' to every static functions in hook.c but complie with -O0, to see if the bug can be reproduced in this way, but nothing happen, this make me doubt being inline is not the culprit. Any inspire is great appreciated! Best wishes from a curiosity cat :) -- Regards, Qian Hong - http://www.winehq.org
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Am 19.04.2013 19:13, schrieb Qian Hong: On Sat, Apr 20, 2013 at 12:52 AM, Dan Kegel d...@kegel.com wrote: I suspect this is a real fix, and there is no gcc bug. Thanks for clarify. Curiosity killed the cat, what is the theory behind this patch? I tried explicitly add 'inline' to every static functions in hook.c but complie with -O0, to see if the bug can be reproduced in this way, but nothing happen, this make me doubt being inline is not the culprit. Any inspire is great appreciated! Best wishes from a curiosity cat :) the inline keyword is meanwhile just a hint for the compiler, it doesn't need to inline it
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Fri, Apr 19, 2013 at 10:38 AM, Dan Kegel d...@kegel.com wrote: Hooking is a fragile business. Somebody somewhere is probably making assumptions about how hooking works (like, how many stack frames are pushed), and inlining call_hook_proc probably violates one of those assumptions. That said, this is all a big guess. It would be nice to have a test that demonstrated the problem. It's up to Alexandre to decide whether making a couple apps happier is evidence enough that disabling inlining of this one function is a good idea. - Dan
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Dan Kegel d...@kegel.com writes: On Fri, Apr 19, 2013 at 10:38 AM, Dan Kegel d...@kegel.com wrote: Hooking is a fragile business. Somebody somewhere is probably making assumptions about how hooking works (like, how many stack frames are pushed), and inlining call_hook_proc probably violates one of those assumptions. That said, this is all a big guess. It would be nice to have a test that demonstrated the problem. It's up to Alexandre to decide whether making a couple apps happier is evidence enough that disabling inlining of this one function is a good idea. It's not. Please take the time to understand the problem and fix it properly. -- Alexandre Julliard julli...@winehq.org
