Re: [PATCH] wined3d: Reduce console flood with an Ogre3D Game
On 22 January 2012 19:44, Detlef Riekenberg <wine.dev(a)web.de> wrote:
- if (usage & ~handled) + static DWORD reported_once; + + if (usage & ~(handled | reported_once)) + { + reported_once |= (usage & ~handled); FIXME("Unhandled usage flags %#x.\n", usage & ~handled); + } I don't think so.
On Sun, 2012-01-22 at 19:53 +0100, Henri Verbeet wrote:
On 22 January 2012 19:44, Detlef Riekenberg <wine.dev(a)web.de> wrote:
- if (usage & ~handled) + static DWORD reported_once; + + if (usage & ~(handled | reported_once)) + { + reported_once |= (usage & ~handled); FIXME("Unhandled usage flags %#x.\n", usage & ~handled); + } I don't think so.
Sorry, I have no Idea, what objections do you have. The code works. Without my Patch, the console is flooded with: 0024:fixme:d3d:resource_check_usage Unhandled usage flags 0x8. With my Patch, the message is printed only once. (The app is using native d3dcompiler_42 and d3dx9_42) -- By by ... Detlef
On Wed, 25 Jan 2012, Detlef Riekenberg wrote:
On Sun, 2012-01-22 at 19:53 +0100, Henri Verbeet wrote:
On 22 January 2012 19:44, Detlef Riekenberg <wine.dev(a)web.de> wrote:
- if (usage & ~handled) + static DWORD reported_once; + + if (usage & ~(handled | reported_once)) + { + reported_once |= (usage & ~handled); FIXME("Unhandled usage flags %#x.\n", usage & ~handled); + } I don't think so.
Sorry, I have no Idea, what objections do you have.
I don't pretend to know what Henry meant but reported_once is not initialized. It's probably put into a zero-initialized section by the compiler but it looks worrying to me (I believe something like this has been debated on the Linux kernel mailing list). I did not try to check the bit manipulations. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ If you think the whole world revolves around you, quit staring at the GPS display while driving.
2012/1/25 Francois Gouget <fgouget(a)free.fr>:
On Wed, 25 Jan 2012, Detlef Riekenberg wrote:
On Sun, 2012-01-22 at 19:53 +0100, Henri Verbeet wrote:
On 22 January 2012 19:44, Detlef Riekenberg <wine.dev(a)web.de> wrote:
- if (usage & ~handled) + static DWORD reported_once; + + if (usage & ~(handled | reported_once)) + { + reported_once |= (usage & ~handled); FIXME("Unhandled usage flags %#x.\n", usage & ~handled); + } I don't think so.
Sorry, I have no Idea, what objections do you have.
I don't pretend to know what Henry meant but reported_once is not initialized. It's probably put into a zero-initialized section by the compiler but it looks worrying to me (I believe something like this has been debated on the Linux kernel mailing list).
I did not try to check the bit manipulations.
-- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ If you think the whole world revolves around you, quit staring at the GPS display while driving.
static variables are zero-initialized by default I don't think that's the problem. Perhaps it's because with such a patch, only the first unhandled flag will be reported and not others (wich can be of a different value), so using a bit mask to only report once every unhandled flags will be better ? -- Nicolas Le Cam
2012/1/25 Nicolas Le Cam <niko.lecam(a)gmail.com>:
2012/1/25 Francois Gouget <fgouget(a)free.fr>:
On Wed, 25 Jan 2012, Detlef Riekenberg wrote:
On Sun, 2012-01-22 at 19:53 +0100, Henri Verbeet wrote:
On 22 January 2012 19:44, Detlef Riekenberg <wine.dev(a)web.de> wrote:
- if (usage & ~handled) + static DWORD reported_once; + + if (usage & ~(handled | reported_once)) + { + reported_once |= (usage & ~handled); FIXME("Unhandled usage flags %#x.\n", usage & ~handled); + } I don't think so.
Sorry, I have no Idea, what objections do you have.
I don't pretend to know what Henry meant but reported_once is not initialized. It's probably put into a zero-initialized section by the compiler but it looks worrying to me (I believe something like this has been debated on the Linux kernel mailing list).
I did not try to check the bit manipulations.
-- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ If you think the whole world revolves around you, quit staring at the GPS display while driving.
static variables are zero-initialized by default I don't think that's the problem. Perhaps it's because with such a patch, only the first unhandled flag will be reported and not others (wich can be of a different value), so using a bit mask to only report once every unhandled flags will be better ?
-- Nicolas Le Cam
I really shouldn't send mail until I get my third coffee ... sorry for the useless noise. -- Nicolas Le Cam
On 25 January 2012 01:25, Francois Gouget <fgouget(a)free.fr> wrote:
I don't pretend to know what Henry meant but reported_once is not initialized. It's probably put into a zero-initialized section by the compiler but it looks worrying to me (I believe something like this has been debated on the Linux kernel mailing list).
Variables with static storage duration (mostly globals and things marked static) are zero-initialized before program startup unless explicitly initialized. That also means they are typically placed in .bss, which I guess is what you're referring to with the lkml discussion. The code looks like it would do what was intended to me. The problem I have with it, and I'm pretty sure I've mentioned this before, is that reducing debug output shouldn't be a goal on its own. If you're a user, and the messages make you feel scared/sad/whatever, just use WINEDEBUG=-all, or even WINEDEBUG=-d3d if the other messages do make you happy. If you're a developer, just fix the actual problem already.
On Wed, Jan 25, 2012 at 02:03:04PM +0100, Henri Verbeet wrote:
The code looks like it would do what was intended to me. The problem I have with it, and I'm pretty sure I've mentioned this before, is that reducing debug output shouldn't be a goal on its own. If you're a user, and the messages make you feel scared/sad/whatever, just use WINEDEBUG=-all, or even WINEDEBUG=-d3d if the other messages do make you happy. If you're a developer, just fix the actual problem already.
There's another, similar FIXME in wined3d_swapchain_set_gamma_ramp() that floods on certain games. These messages are extremely annoying when trying to check for other, more interesting messages. What about changing them to WARN so that they don't print to the console by default? Everyone knows there are workarounds, but is anyone really gaining anything by having these flood the console? Andrew
Am 25.01.2012 14:40 schrieb "Andrew Eikum" <aeikum(a)codeweavers.com>:
What about changing them to WARN so that they don't print to the console by default? Everyone knows there are workarounds, but is anyone really gaining anything by having these flood the console? I agree with this sentiment, althougj Henri's point is valid too.
The specific issue here is that proper handling of the WRITEONLY flag isn't obvious, so a proper fix hasn't happened yet and is unlilely to happen soon. The negative effects of not handling writeonly are small too, so besides the fixme spam this issue is low priority IMO
participants (6)
-
Andrew Eikum -
Detlef Riekenberg -
Francois Gouget -
Henri Verbeet -
Nicolas Le Cam -
Stefan Dösinger