[Approved] - Added proper horizontal scrollbar to filmstrip

Discussion in 'Archive' started by pilehave, January 27, 2011.

  1. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
    For some reason, a vertical scrollbar was attached to filmstrip. I guess someone never got around to finishing it, the code was really weird :D

    I also decided to fix the horizontal scrollbar, which did not scale when the window did. So the scrollbar would be just as high and wide in 720x576 as in 1920x1080. Not good.

    I have added the following new attributes to filmstrip facade:

    scrollbarYOff will move the scrollbar a number of pixels up or down, calculated from the posY of the filmstrip-control
    scrollbarWidth and scrollbarHeight are added, and can be used to set the size of the scrollbar (just as in coverflow)

    The scrollbar now allows for the user to click and hold to navigate the filmstrip or to point and click to move a percentage left or right. It works like a normal scrollbar in any browser and same way as it does in listview etc.

    While I was at it, I also fixed a render problem which made images in filmstrip disappear when the window was resized.

    I have attached my patch and a screenshot of filmstrip.

    EDIT New patch attached, I moved scrollbarfixes to this thread to keep things clean:
    https://forum.team-mediaportal.com/...tical-horizontal-scrollbars-93059/#post710074

    EDIT 2 Removed scrollbarXOff as I think it should just position in the centre of the filmstrip-control. Also removed the option to show/hide scrollbar, as this IMHO should be up to the settings in MediaPortal Configuration to decide.
    New patch attached, old removed.

    scrollbar_filmstrip.jpg
    Example with a screenwide filmstrip



    example2.jpg
    Example with a filmstrip positioned to the right. Scrollbar still centered
     

    Attached Files:

    • Like Like x 3
  2. Google AdSense Guest Advertisement



    to hide all adverts.
  3. tourettes
    • Premium Supporter

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    That makes it sound like there will be ugly copy & paste code which we don't want to have. Instead you should create a common scrollbar class that both views are utilizing (and maybe some other places as well).
     
  4. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
    Please review the code before judging...
     
  5. tourettes
    • Premium Supporter

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    Well if you already state that the code was copied mostly (or even partly) from a different class then it is clearly an indicator that there is a design issue.
     
  6. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
    Having a bad morning? If you don't even bother looking at the code, why are you even here?
     
  7. tourettes
    • Premium Supporter

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    Sometimes I wonder that myself as well...


    About the patch - I dont have access to source codes currently so I have only your word that lot of code was copied. Shouldn't that be a sign / indicator of a bad design? Lately we have been cleaning a lot of copy & paste code in MP1.2.0b and we dont want to introduce new one. Patch / feqature itself might be a good one but the approach should be a different one - trying to reduce the copy & paste code when ever possible if new GUI components are added (or anything else). It will make the code much more easy to maintain when there is no copy & paste code around.
     
  8. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
    OK, maybe you emphasize the word copy a bit too much, it was just a question of getting the scrollbar to calculate the percentage correctly. Andy already did these 5 lines or so in coverflow.

    Before my patch, the code for implementing the horizontal scrollbar was this:

    Code (Text):
    1. ///TODO: add horz. scrollbar to filmstrip
    Wow...quality coding, must say that!

    Now it works.

    The part I copied was this:

    Code (Text):
    1.  
    2. +                // We require mouse support for the scrollbar to respond properly.  Temporarily bypass the global setting to allow
    3. +                // the action to work for us.
    4. +                bool ms = GUIGraphicsContext.MouseSupport;
    5. +                GUIGraphicsContext.MouseSupport = true;
    6. +
    7. +                _horizontalScrollbar.OnAction(action);
    8. +                int index = (int)((_horizontalScrollbar.Percentage / 100.0f) * _listItems.Count);
    9. +                _offset = index;
    10. +                _cursorX = 0;
    11. +                OnSelectionChanged();
    12. +
    13. +                GUIGraphicsContext.MouseSupport = ms;
    14.  
    I don't see how you would put that in a separate class for both filmstrip and coverflow to utilize, but if you feel that's the right way, then you are more than welcome to do so.

    Alternatively, just remove the entire existing scrollbar part of MediaPortal, because it is full of bugs and utterly useless.
     
  9. tourettes
    • Premium Supporter

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    I think we had some communication issue. From the 1st post I got the feeling that majority of the code was copied.

    Having bad quality code in current revision is not an excuse to have new code with low quality (note that this is not related to your patch at all, but just in general that the current horrbile code quality level is some place cannot be used an excuse to add more klugde next to it :)).

    You are right, there is no need for having a common code for such. Althou the MouseSupport seems to be a hack and should be better handled in the MP in general :)
     
  10. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
    I have removed the mouse-bypass and I also fixed the vertical scrollbar. It makes sense to respect the users setting on mousecontrols.

    For some reason, the vertical scrollbar's 100% was the number of items + 1, so the handle would never reach the bottom. Same problem in TVGuide, so I'll also provide a fix for that one as well. It also never scrolled past the number of channels when in singlechannel-view, so if you (like me) only have 8 channels, scrolling in singlechannel-view would not update the scrollbar beyond 8 lines...

    I'll drop a patch sometime in this week, I just need to fix something else in TVGuide; currently, when you use "PgUp" when in singlechannel-view and focused on one of the time buttons, you can scroll into a deadend...
     
  11. pilehave
    • Premium Supporter

    pilehave Community Skin Designer

    Joined:
    April 2, 2008
    Messages:
    2,566
    Likes Received:
    500
    Gender:
    Male
    Occupation:
    Technical consultant, marketing
    Location:
    Hornslet
    Ratings:
    +517 / 0
    Home Country:
    Denmark Denmark
Loading...

Users Viewing Thread (Users: 0, Guests: 0)

  1. This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
    By continuing to use this site, you are consenting to our use of cookies.
    Dismiss Notice
  • About The Project

    The vision of the MediaPortal project is to create a free open source media centre application, which supports all advanced media centre functions, and is accessible to all Windows users.

    In reaching this goal we are working every day to make sure our software is one of the best.

             

  • Support MediaPortal!

    The team works very hard to make sure the community is running the best HTPC-software. We give away MediaPortal for free but hosting and software is not for us.

    Care to support our work with a few bucks? We'd really appreciate it!