[mythtv] [PATCH] Display Resolution switching

John Patrick Poet john at BlueSkyTours.com
Sat Jul 31 04:28:32 EDT 2004


Isaac Richards wrote:

>On Thursday 29 July 2004 12:07 am, John Patrick Poet wrote:
>  
>
>>This is version 3 of my Display Resolution switching patch.
>>
>>The patch enables automatic switching to different display resolutions
>>based on the dimensions of the video being played.  If USING_XRANDR is
>>enabled in settings.pro, a new config screen will show up under
>>[Appearance].  This config screen allows you to specify the display
>>resolution to use for the GUI, a default video display resolution, and
>>two "override" video display resolutions.
>>    
>>
>
>This is looking much better, but still has some issues I'd like to see taken 
>care of before I commit:
>
>- It won't compile at all without xrandr
>  
>

If xrandr is not available on the machine, it will not compile?

>- It won't compile at all if using_x11 isn't defined
>  
>

Damn.  I did test that, but I must have modified libmyth.pro sometime 
afterwards.

>- Why is everything in DisplayRes static?  I don't see a reason for that.
>  
>

The way I wrote it initially, it had to be static.  Another way to 
handle it, would have been to make it a singleton class, but I don't 
like making the "main" program do extra work to instantiate/use a 
class.  With it as a member of MythContext it actually does not have to 
be done either way...

>- Why is the DisplayRes class a public member of the context?
>

 I made it a member of MythContext because that is the primary place it 
is used, and other "classes" would have easy access to it.

>  I think it'd 
>live best as a singleton class (see some recent mediamonitor changes),
>

Okay, I will make it a singleton class.

> but at 
>the least it should be a private member with an accessor function.
>  
>

That data that should be private, is private within the DisplayRes 
class, so I did not see a reason to use an accessor function.

>Other suggestions:
>- I'd make the display res class grab its own settings, and not feed em in 
>through Activate() & addRes().
>  
>

I did that so it would not have to know anything about gContext -- I was 
trying to cut down on the dependencies.  I will change it.

>- I would _really_ prefer it if your patch followed the rest of the style 
>conventions ('if' is not a function, 4 space indents, etc).  Otherwise, I 
>have to spend time fixing things as I apply it.
>  
>

I actually did try.  While working on this, I even went into my .emacs 
file and changed the default to 4 space indents.  I will re-audit the 
code and try to find the places I missed.

>Isaac
>_______________________________________________
>  
>

Thanks,

John



More information about the mythtv-dev mailing list