Tue 24 Jan 2012 08:35:51 PM UTC, comment #11:
Thanks Rik.
I was wondering about the coding style part.
I have some more changes to graphics.cc. I'll review my coding before pushing.
Ben
|
Tue 24 Jan 2012 05:18:44 PM UTC, comment #10:
Good patch Ben. I can see you're going to be a full-fledged C++ programmer soon.
I made some style changes and checked them in here (http://hg.savannah.gnu.org/hgweb/octave/rev/08779abcb640). I'll explain what I did so you can see the method in my madness.
I like to carryover the Octave spacing convention for functions and indexing that exists in m-files. To whit, use a space between the function name and the opening parenthesis, but cuddle the parenthesis when the name is a variable and the operation is indexing. Here is a good example from earlier in graphics.cc.
This is completely optional, but I like using the in-place operators such as *=, +=, etc. In m-file implementations these are 3X faster. In C++, gcc is probably smart enough to optimize away the temporary. However, who wants to take a chance?
The graphics property framework has a front end and a back end. The front end (set routines) has to deal with caseless strings because people will type anything in any format. However, the values are always stored in a standardized format and the back end (get routines) always return a lower case version of the property in question. I changed some of your patch to use std::string where it was possible. The equality operator is overloaded for strings so this allows the code to be simplified.
There's no way that you would know about it, but the swapping of temporary values happens so frequently that C++ includes a routine for it in the std library. I rewrote the following.
Finally, I added the 'const' keyword to your declarition of papertype conversion values. The compiler might have figured out that you only assigned to these once. But, it is better to be explicit and tell the compiler that. In this case, it can then do all the multiplication and division only once at compile time and the magic values are stored in the code. Otherwise, there is a chance that these values are computed again every time the function is called.
This may look like a lot, but it really isn't. These are style issues only and the code works without them.
|
Tue 24 Jan 2012 01:32:40 PM UTC, comment #9:
I've pushed a changeset.
I'll close this. If something is wrong with the changeset, this can be reopened.
|
Tue 24 Jan 2012 03:02:43 AM UTC, comment #8:
I've attached a changeset that works for me. If there are no objections, I'll push it to the default branch tomorrow.
(file #24879)
|
Tue 24 Jan 2012 01:46:28 AM UTC, comment #7:
I think I found the problem. I'll post a changeset once I've done more testing.
|
Tue 24 Jan 2012 01:27:19 AM UTC, comment #6:
I'm close to a fix, but am unable to figure out how to add an updater for the paperorientation to the sources.
I'm attached a patch (paperorientation.patch) that ends with the error below.
I don't have a solid understanding for what needs to be done to add an updated to the figure::properties. So, I just tried to copy what was done for papertype.
(file #24878)
|
Mon 23 Jan 2012 01:02:26 AM UTC, comment #5:
I revived my old m-file implementation and reviewed the updating portions in graphics.cc.
At the present, the m-file and c++ implementations interfere with each other.
It looks to me as if the proper think to do is to do the fix in graphics.cc
|
Sun 22 Jan 2012 10:36:42 PM UTC, comment #4:
Opps, I submitted before I was done.
In 2008, I looked at implementing all the listeners as m-files. The problem I encountered was how to go about registering the listeners as objects were created.
In addition, the listeners tend to recursive. For example, changing papertype triggers a change to papersize, changing papersize triggers a change to papertype, & orientation. So it is necessary to suppress recursion while the listener is updating.
Since there is a partial c++ side implementation for the paper properties, it is either necessary to extend the c++ code, or to remove it and rely upon m-files entirely.
In either event, I still have my work from 2008. I'll dig it out and see if I can separate the part for the paper properties.
|
Sun 22 Jan 2012 10:24:47 PM UTC, comment #3:
I should have been more thorough in my original comment. There is already some listeners in place for the paper properties.
However, the listener doesn't always to the right thing.
The papertype should be automatically detected.
In addition to the papertype, the orientation should also be detected.
|
Sun 22 Jan 2012 08:10:40 PM UTC, comment #2:
What needs to be done in the C++ code? Do you want to add a permanent listener on "paperorientation" that updates papersize?
Just brainstorming, is the following approximately what you are after?
|
Thu 19 Jan 2012 03:02:47 PM UTC, comment #1:
At the moment the print() function assumes landscape means width > length.
The gs_papersize() function included in _print_parse_opts_.m has some information that may be relevant to fixing this, and should be removed after the c++ side is fixed.
|
Thu 19 Jan 2012 03:02:06 PM UTC, original submission:
The following shows that, unlike matlab, octave does not update papersize from paperorientation:
get(gcf, 'paperorientation')
ans = portrait
get(gcf, 'papersize')
ans =
8.50 11.00
set(gcf, 'paperorientation', 'landscape')
get(gcf, 'papersize')
ans =
8.50 11.00
|