From: Philippe M. <phi...@gm...> - 2008-01-27 17:01:04
|
Hi all, two weeks ago I filed feature request #1875876 about the wish to have the transport mode persistent. I wanted to challenge myself into doing an implementation, feeling such a feature would be a good way for a former C++ programmer to know better Rosegarden from the inside, and contribute to it. After walking through some of the code of RG, I gave it a try: I "cloned" some of the handling of the ZoomLevel parameter. This means that I have in my RG files a new "Int" property under the "configuration" section, which I called "transportmode". I have two questions on this: 1. is this a good place to store this data? I thought it would be, so that it could have a different value for MIDI and audio projects, which depends on the Composition; and this way it could have, saved into the default document, a startup value. 2. since the transport mode is an enum (TimeDisplayMode) it may not be robust enough to store the value as an Int, since this value depend on its position in the definition of the enum (e.g. BarMode happens to be equals to 2 only because it is the third of the list...). Do you think it is better to store a string description instead, to be mapped at load time to an Int? (e.g. a string "BarMode") Now to my actual problem: everything works fine... except that I can't manage to load the transportmode value from the file. I modified class DocumentConfiguration to store the value into the XML, and this works. But when I try to load it (in class RosegardenGUIApp.cpp, just like ZoomLevel) I get a NoData exception. Is there anything to declare somewhere to have the a value loaded from the XML file? I tried to delve into class RoseXmlHandler but couldn't find anything specific to parameter loading. Thanks for the help and time. -- Best regards, Philippe. |
From: D. M. M. <mic...@ro...> - 2008-01-27 21:34:49
|
On Sunday 27 January 2008, Philippe Macaire wrote: > I wanted to challenge myself into doing an implementation, feeling > such a feature would be a good way for a former C++ programmer to know > better Rosegarden from the inside, and contribute to it. You can't imagine how much I like the sound of that. :) > 1. is this a good place to store this data? I wouldn't have stored it there, but whoever actually gets into coding something first gets a lot of leeway to determine things like this. It's probably fine. > 2. since the transport mode is an enum (TimeDisplayMode) it may not be > robust enough to store the value as an Int, since this value depend on > its position in the definition of the enum (e.g. BarMode happens to be > equals to 2 only because it is the third of the list...). Do you think > it is better to store a string description instead, to be mapped at > load time to an Int? (e.g. a string "BarMode") Depending on the position in an enum like that is easy to break down the road, so it would be best to avoid that, but probably not a deal breaker I wouldn't say. I'm pretty sure I've done something similarly evil at least once, if not more than once, so I probably live in a glass house. > Now to my actual problem: everything works fine... except that I can't > manage to load the transportmode value from the file. I modified class > DocumentConfiguration to store the value into the XML, and this works. > But when I try to load it (in class RosegardenGUIApp.cpp, just like > ZoomLevel) I get a NoData exception. > > Is there anything to declare somewhere to have the a value loaded from > the XML file? I tried to delve into class RoseXmlHandler but couldn't > find anything specific to parameter loading. My memory of how all of this works is too vague to guess what's going wrong without looking at your patch. I know I managed to do that with the track parameters stuff, so I can probably figure out the piece you're missing if I can look at it. -- D. Michael McIntyre |
From: Philippe M. <phi...@gm...> - 2008-01-28 08:16:30
|
On Jan 27, 2008 10:34 PM, D. Michael McIntyre <mic...@ro...> wrote: > > 1. is this a good place to store this data? > > I wouldn't have stored it there, but whoever actually gets into coding > something first gets a lot of leeway to determine things like this. Where else would you store it? Just out of curiosity. > > 2. since the transport mode is an enum (TimeDisplayMode) it may not be > > robust enough to store the value as an Int, since this value depend on > > its position in the definition of the enum (e.g. BarMode happens to be > > equals to 2 only because it is the third of the list...). Do you think > > it is better to store a string description instead, to be mapped at > > load time to an Int? (e.g. a string "BarMode") > > Depending on the position in an enum like that is easy to break down the road, > so it would be best to avoid that, but probably not a deal breaker I wouldn't > say. I think I can spend the time to map it to a string... as soon as the loading problem is resolved. > > Is there anything to declare somewhere to have the a value loaded from > > the XML file? I tried to delve into class RoseXmlHandler but couldn't > > find anything specific to parameter loading. > > My memory of how all of this works is too vague to guess what's going wrong > without looking at your patch. I know I managed to do that with the track > parameters stuff, so I can probably figure out the piece you're missing if I > can look at it. I will clean it up a little bit when I'm home tonight and send it to you. BTW I'm working on stable_1_6. I hope it's OK for you to receive an svn diff on that version. Let me know. -- Best regards, Philippe. |
From: Philippe M. <phi...@gm...> - 2008-01-29 08:04:24
Attachments:
patch-phil.diff
|
On Jan 28, 2008 9:16 AM, Philippe Macaire <phi...@gm...> wrote: > I will clean it up a little bit when I'm home tonight and send it to you. > > BTW I'm working on stable_1_6. I hope it's OK for you to receive an > svn diff on that version. Let me know. Here is the diff of what I've worked out. Please remind that I don't consider it as final (yet) :-) I've put some START Phil / END Phil marks so as to keep track of the changes I made. Comments welcome, of course. -- Best regards, Philippe. |
From: Guillaume L. <gla...@te...> - 2008-02-02 09:32:38
|
On Jan 29, 2008, at 9:04 , Philippe Macaire wrote: > I've put some START Phil / END Phil marks so as to keep track of the > changes I made. Side comment : Svn can tell you who changed what so this is really not needed and actually makes code less readable. -- Guillaume http://telegraph-road.org |
From: Chris C. <ca...@al...> - 2008-01-29 11:46:50
|
On 29/01/2008, Philippe Macaire <phi...@gm...> wrote: > Here is the diff of what I've worked out. Please remind that I don't > consider it as final (yet) :-) I had a look at it (hope you don't mind my springing in, Michael!) -- I have to admit it took me rather a long time to work out what was going wrong. Your setting is actually being saved and restored from the file correctly. The problem is that it is then overwritten with the current state of the dialog before the dialog is updated from the loaded setting. This is because RosegardenGUIApp::initView() calls slotSetPointerPosition explicitly before reaching your new lines of code that set the new mode, and slotSetPointerPosition among other things resets the configuration value from the dialog's current (i.e. default) state. To fix this, all you need to do is move your getTransport()->setNewMode() further up initView so that it happens just before the call to slotSetPointerPosition. Chris |
From: Philippe M. <phi...@gm...> - 2008-01-29 12:10:01
|
On Jan 29, 2008 12:46 PM, Chris Cannam <ca...@al...> wrote: > Your setting is actually being saved and restored from the file > correctly. The problem is that it is then overwritten with the > current state of the dialog before the dialog is updated from the > loaded setting. This is because RosegardenGUIApp::initView() calls > slotSetPointerPosition explicitly before reaching your new lines of > code that set the new mode, and slotSetPointerPosition among other > things resets the configuration value from the dialog's current (i.e. > default) state. > > To fix this, all you need to do is move your > getTransport()->setNewMode() further up initView so that it happens > just before the call to slotSetPointerPosition. Well, sounds like good news! I'll check this out tonight when I'm in front of my Linux box. Thanks a lot, Chris. -- Best regards, Philippe. |
From: Philippe M. <phi...@gm...> - 2008-01-30 08:55:55
|
On Jan 29, 2008 12:46 PM, Chris Cannam <ca...@al...> wrote: > To fix this, all you need to do is move your > getTransport()->setNewMode() further up initView so that it happens > just before the call to slotSetPointerPosition. It did work! I surely need to know better the flow of control in RG. I did the following change in RosegardenGUIApp::initView(): // Transport setup // //START Phil int transportMode = m_doc->getConfiguration(). get<Int>(DocumentConfiguration::TransportMode); getTransport()->setNewMode(transportMode); //END Phil slotEnableTransport(true); // other getTransport()->... calls to set time signature, tempo, etc. //MODIF Phil: moved from above slotEnableTransport() // set the pointer position // slotSetPointerPosition(m_doc->getComposition().getPosition()); I tried to move the last line so as to have the setNewMode() called before slotSetPointerPosition() but kept together with other Transport related calls, and apparently it works. I'm now trying to make it more robust, and string-based so as to avoid hardcoding the enum value in the XML file. I will get back to you then. Thanks for the help. -- Best regards, Philippe. |
From: Guillaume L. <gla...@te...> - 2008-02-02 10:16:36
|
On Jan 29, 2008, at 9:04 , Philippe Macaire wrote: > I've put some START Phil / END Phil marks so as to keep track of the > changes I made. Side comment : this is really not needed and actually (svn provides tools to do just that -- Guillaume http://telegraph-road.org |
From: Guillaume L. <gla...@te...> - 2008-02-02 11:22:16
|
On Feb 2, 2008, at 11:16 , Guillaume Laurent wrote: > > On Jan 29, 2008, at 9:04 , Philippe Macaire wrote: > >> I've put some START Phil / END Phil marks so as to keep track of the >> changes I made. > > Side comment : this is really not needed and actually (svn provides > tools to do just that (sorry for the double post, my hoster is acting strangely) -- Guillaume http://telegraph-road.org |
From: Philippe M. <phi...@gm...> - 2008-02-03 16:57:32
|
Hi Guillaume, On Feb 2, 2008 11:16 AM, Guillaume Laurent <gla...@te...> wrote: > > On Jan 29, 2008, at 9:04 , Philippe Macaire wrote: > > > I've put some START Phil / END Phil marks so as to keep track of the > > changes I made. > > Side comment : this is really not needed and actually (svn provides > tools to do just that Don't worry, they won't make it to the repository: it's just a way for me to keep track changes I made (sorry, it's my first time contributing to an Open Source project). -- Best regards, Philippe. |
From: Philippe M. <phi...@gm...> - 2008-02-03 17:13:14
|
On Jan 30, 2008 9:55 AM, Philippe Macaire <phi...@gm...> wrote: > On Jan 29, 2008 12:46 PM, Chris Cannam <ca...@al...> wrote: > > To fix this, all you need to do is move your > > getTransport()->setNewMode() further up initView so that it happens > > just before the call to slotSetPointerPosition. > > It did work! I surely need to know better the flow of control in RG. > [,,,] > > I'm now trying to make it more robust, and string-based so as to avoid > hardcoding the enum value in the XML file. I will get back to you > then. I finished off the patch saving the data as a readable string ("BarMode", "RealMode", etc.), but I'm still cranking through a problem when loading the default document which does not switch to the right transport mode. I think I'm still missing something regarding the control flow of GUI objects at startup because I've had the same problem with an alternate patch I made handling the configuration data with KConfig. I'm trying to drill down with a debugger, but it all takes time, and I have a lot of things to learn (and very little time). Sorry if all this is taking so long, for such a simple feature. When is version 1.7 expected to come out? -- Best regards, Philippe. |
From: D. M. M. <mic...@ro...> - 2008-02-03 18:29:04
|
On Sunday 03 February 2008, Philippe Macaire wrote: > Sorry if all this is taking so long, for such a simple feature. When > is version 1.7 expected to > come out? I've spent an insane number of hours over the past few weeks digging around to track down simple bugs, so I have barely accomplished anything on my staff group hacking branch, and I really want to get that done before 1.7.0, and then get one or more tutorials together that show it off. (Smaller vision than the grand ideal we once talked about, but I have big ideas how much better than nothing it will be, and am really looking forward to using this. I'm currently stumped with LilyPond itself. Once I know what to say in LilyPond, the rest of it should follow in a pretty straightforward manner, but I just haven't found the time to sit down with the docs and puzzle it out.) I've been pulling together the commit logs today, and updating the wiki [1] in preparation for a release, but there are a number of documentation tasks that need to be done before then, Chris is still hoping to fix a notation rendering bug, and Jaakko is still working on his marker export thing. Then I think we really must do something in the way of sorting out the track headers rendering font (and it would be really nice to make them more compact too.) I think we should start trying to pull together for a release "very soon" in Rosegarden's scale of time, which means I'm currently thinking about the first of March, being realistic about how much time I have to do the stuff I really want done. It's always a bit of a juggle trying to decide where to draw the line on what makes it in, and what must be put off, but I would deeply like to see the staff stuff along with all the new transpose stuff Arnout and I (mostly my thinking and his excellent and timely doing) have been working on, plus the newly revamped grace notes (HUZZAH!!!) as part of a package of new features for the best notation-friendly release of Rosegarden yet. We might ought to do a 1.6.2 in the meantime, with some significant bug fixes, but since uptake of 1.6.1 was not very good, I think I'm inclined to just save it all for 1.7.0. 1. http://rosegarden.wiki.sourceforge.net/Things+To+Do+for+1.7.0 -- D. Michael McIntyre |
From: Chris C. <ca...@al...> - 2008-02-05 10:45:27
|
On Sunday 03 February 2008 17:13, Philippe Macaire wrote: > I finished off the patch saving the data as a readable string > ("BarMode", "RealMode", etc.), but I'm still cranking through a > problem when loading the default document which does not > switch to the right transport mode. I think I'm still missing > something regarding the control flow of GUI objects at startup Do feel free to send more patches to the list if you'd like someone else to look at them. Chris |
From: Philippe M. <phi...@gm...> - 2008-02-05 11:02:37
|
On Feb 5, 2008 11:43 AM, Chris Cannam <ca...@al...> wrote: > On Sunday 03 February 2008 17:13, Philippe Macaire wrote: > > I finished off the patch saving the data as a readable string > > ("BarMode", "RealMode", etc.), but I'm still cranking through a > > problem when loading the default document which does not > > switch to the right transport mode. I think I'm still missing > > something regarding the control flow of GUI objects at startup > > Do feel free to send more patches to the list if you'd like someone else > to look at them. I will, Chris, as soon as I've correctly switched to trunk (I'm still working with version stable_1_6). It shouldn't be a big deal, but I have little time lately. Thanks. -- Best regards, Philippe. |