Some improvement and addition

Dalan94 commented 9 years ago

Here is the changes that I need for my project. I hope that it fit you. Don't hesitate to not accept or change things.

elvismt commented 9 years ago

Seems good, I like these changes . The only thing I wouldchange is: leave SLOPE_LINE as a symbol like circles and squares, etc.. draw line + symbol is a good thing but it can be done optimaly by creating a dedicated __slopedraw... func that traverses the data set only once. I have also made some modifications so I'll try to merge them.

elvismt commented 9 years ago

I decided to change the names of primitive types: slope_scene_t -> slope_figure_t why: scene has a special meaning in scene/view model slope_data_t -> slope_item_t why: item like axis and legend also derive from this type and they are not "the data" of interest, just "items" that are shown. Take a look at my repo. If you change this in yours and rename the files accordingly (e.g. data.h -> item.h) to avoid merge conflicts, I will sync our repos with your modifications and my own recent changes.

Dalan94 commented 9 years ago

I agree with you about the names. I don't know enough cairo to create a new draw function…

elvismt commented 9 years ago

Ok, I will create the line+simbol draw function, just change the names slope_scene_t -> slope_figure_t. slope_data_t -> slope_item_t. and revert the change where you droped the SLOPE_LINE symbol. this way will be easy to me to merge your contributions. Thank you.

Dalan94 commented 9 years ago

It's done

Dalan94 commented 9 years ago

Thanks for adding most of my contribution. I have only a few comment. I understand that you prefer using width and height in the PDF writting but why do you keep the slope_paper_size_t and slope_color_name_t enum ? Furthermore you keep all the color in the slope_color_name_t enum but only a few in the slope_color_set_name function and none in the __slope_item_parse_color function. I think that a lot of color and a automatic color is very useful (in any way I need it).

elvismt commented 9 years ago

Thanks for your interest.

Well regarding the color, you are right. I think I stoped working on that in the middle of the change but I will complete addition of them to the set_color_name func.
In the PDF export function I really prefer using width and height because ine will rarely use an entire A4 for a chart, but prbably embed a smaller one in a pdf document and the visual aspect of a a4 sized one is not pretty In my opinion. Aditionaly the current form is compliant with form of the other write_to funcs and I like it.
Regarding the automatic attribution of color I prefer to leave it to the client application rather that enforce it on the library, so move this code to your app please.

Additionly, have you see the other changes? Now we have a legend and the GtkWidget has mouse zooming.

On Wed, Jun 17, 2015 at 10:08 AM, Dalan94 notifications@github.com wrote:

Thanks for adding most of my contribution. I have only a few comment. I understand that you prefer using width and height in the PDF writting but why do you keep the slope_paper_size_t and slope_color_name_t enum ? Furthermore you keep all the color in the slope_color_name_t enum but only a few in the slope_color_set_name function and none in the __slope_item_parse_color function. I think that a lot of color and a automatic color is very useful (in any way I need it).

— Reply to this email directly or view it on GitHub https://github.com/elvismt/slope/pull/1#issuecomment-112793496.

Dalan94 commented 9 years ago

Thanks for the color, don't forget the function __slope_item_parse_color that you didn't mention it, otherwise we cannot use the color
I completely understand for the pdf export, so you can delete the slope_paper_size_t and slope_color_name_t enum which are not used
OK I will created the automatic color in my app
Yes I see the changes. It' great. It's very easy to use it with GTK without any cairo knowledge and the zoom is also very useful and easy to use. The legend is great and the text is better with pango. I only notice that the chart title is a little to high and touch the top of the surface.
Are you always considering create the line+symbol draw function ?

elvismt commented 9 years ago

Yes, I have all of that in mind but I'm a little busy now, and as you can probably guess, slope isn't my main business. It is a free time activity. That said, these polishings are on track for next week.

On Wed, Jun 17, 2015 at 3:05 PM, Dalan94 notifications@github.com wrote:

Thanks for the color, don't forget the function __slope_item_parse_color that you didn't mention it, otherwise we cannot use the color

I completely understand for the pdf export, so you can delete the slope_paper_size_t and slope_color_name_t enum which are not used

OK I will created the automatic color in my app

Yes I see the changes. It' great. It's very easy to use it with GTK without any cairo knowledge and the zoom is also very useful and easy to use. The legend is great and the text is better with pango. I only notice that the chart title is a little to high and touch the top of the surface.

Are you always considering create the line+symbol draw function ?

— Reply to this email directly or view it on GitHub https://github.com/elvismt/slope/pull/1#issuecomment-112897177.

Dalan94 commented 9 years ago

I understand perfectly. Take your time. Thanks for your work.

elvismt commented 9 years ago

Well, except for the text going out of the figure boundaries, it's all done. Full pango support will take a little more effort so I strongly suggest that you use cairo's toy API untill we are done with pango. The toy api is now default in build script. Enjoy it and thank you for the feedback.

Dalan94 commented 9 years ago

Thanks. You only miss to add this in the slope_color_set_name function :

        case SLOPE_PURPLE:
            slope_color_set(color, 0.5, 0.0, 0.5, 1.0);
            break;
        case SLOPE_OLIVE:
            slope_color_set(color, 0.5, 0.5, 0.0, 1.0);
            break;
        case SLOPE_TEAL:
            slope_color_set(color, 0.0, 0.5, 0.5, 1.0);
            break;
        case SLOPE_ORANGE:
            slope_color_set(color, 1, 0.65, 0.0, 1.0);
            break;

Again thanks for your great job.

elvismt commented 9 years ago

Ok, will be fixed.

Dalan94 commented 9 years ago

Thanks. I just notice that the slope_figure_write_to_png function doesn't return the status of the operation like the others slope_figure_writeto function.

Dalan94 commented 9 years ago

I also noticed that you forgot SLOPE_BEGIN_DECLS and SLOPE_END_DECLS in the slope.h file.

elvismt commented 9 years ago

Up to next nightly build. Keep watching for news.

[image: LSBD] http://www.lsbd.ufc.br/

Elvis Teixeira

DEVELOPER

[image: LSBD]elvis.teixeira@lsbd.ufc.br

[image: LSBD]+55 (85) 3366 9235

[image: LSBD]+55 (85) 9135 3008

[image: LSBD]elvis.teixeira

On Fri, Jun 26, 2015 at 9:24 AM, Dalan94 notifications@github.com wrote:

I also noticed that you forgot SLOPE_BEGIN_DECLS and SLOPE_END_DECLS in the slope.h file.

— Reply to this email directly or view it on GitHub https://github.com/elvismt/slope/pull/1#issuecomment-115659626.

elvismt commented 9 years ago

I have made a major change, xyitem will be called funcplot. Pless update it in your application. (I promess not to keep renaming things from now on). Also, I am new to git so I didn't fixed the merge conflicts in this pull request, rather put your contributions by hand but next time I will do this properly so that github tracks your contribution. So feel free to push, you just please ask me about the changes before you push them.

Dalan94 commented 9 years ago

OK fine

bytebrew / slope

Some improvement and addition #1