Re: [PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Henri Verbeet]

On 22 January 2012 19:44, Detlef Riekenberg  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.

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Detlef Riekenberg]

On Sun, 2012-01-22 at 19:53 +0100, Henri Verbeet wrote:
> On 22 January 2012 19:44, Detlef Riekenberg  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)

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Francois Gouget]

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  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.

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Nicolas Le Cam]

2012/1/25 Francois Gouget :
> 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  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               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 ?

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Nicolas Le Cam]

2012/1/25 Nicolas Le Cam :
> 2012/1/25 Francois Gouget :
> > 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  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               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.

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Henri Verbeet]

On 25 January 2012 01:25, Francois Gouget  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.

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Andrew Eikum]

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

[PATCH] wined3d: Reduce console flood with an Ogre3D Game

[Stefan Dösinger]

Am 25.01.2012 14:40 schrieb "Andrew Eikum" :
> 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