Inset feature #50

serzhiio · 2019-10-10T19:57:13Z

        let mut price_chart = ChartBuilder::on(&price_area)
            .x_label_area_size(-15)
            .y_label_area_size(40).inset_y_labels()
            .right_y_label_area_size(40).inset_right_y_labels()
            .build_ranged(from..to, min..max).unwrap()

I've found negative values sometimes useful so i enabled i32 instead of u32 in label_size methods.
Hope it helps!

update

…ive margin), title for ShapeStyle (better backend drawing customizing), some typos

codecov · 2019-10-10T20:20:09Z

Codecov Report

Merging #50 into master will decrease coverage by 0.48%.
The diff coverage is 37.66%.

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   49.45%   48.97%   -0.49%     
==========================================
  Files          43       43              
  Lines        3195     3257      +62     
==========================================
+ Hits         1580     1595      +15     
- Misses       1615     1662      +47

Impacted Files	Coverage Δ
src/coord/mod.rs	`22.22% <ø> (ø)`	⬆️
src/drawing/backend.rs	`50% <0%> (-1.17%)`	⬇️
src/chart/builder.rs	`50.47% <25%> (-12.49%)`	⬇️
src/style/mod.rs	`72% <33.33%> (-12.22%)`	⬇️
src/drawing/area.rs	`83.89% <36.36%> (-3.41%)`	⬇️
src/chart/context.rs	`85.07% <68.75%> (-2.43%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a70245...f946699. Read the comment docs.

38 · 2019-10-11T02:18:24Z

src/chart/builder.rs

@@ -18,7 +18,8 @@ pub enum LabelAreaPosition {
 /// With the hlep of this object, we can convert a basic drawing area into a chart context, which
 /// allows the high-level chartting API beening used on the drawing area.
 pub struct ChartBuilder<'a, 'b, DB: DrawingBackend> {
-    label_area_size: [u32; 4], // [upper, lower, left, right]
+    label_area_size: [i32; 4], // [upper, lower, left, right]
+    label_area_inset: [bool; 4],


Just want to discuss with you. Since what I am planning to do is allow developer give a negative label area size, and it will automatically inverts the direction of the labels. Is there any particular reason for explicitly setting this ?

That was first option i wanted to implement, but then i realized that negative width is usefult itself. It gives more control over drawing areas. It allows to overlay one drawing area onto one with negative size.

38

Hi @serzhiio ,

Thank you so much for the PR. In general it looks awesome to me. But I may have a few questions and some concerns about the change.

It would be helpful if you can give me more details.

Thanks again!

38 · 2019-10-11T02:33:39Z

src/chart/builder.rs

+        for (id, (_, size)) in self.label_area_inset.iter().zip(self.label_area_size.iter()).enumerate().filter(|(_,(inset, size))| **inset && **size != 0) {
+            let area = splitted[4].as_ref().unwrap();
+            let (w, h) = area.dim_in_pixel();
+            let mut new_area = match id {


What I am planning to do is uses the drawing::area::shrink function if the sizes are negative to construct the label area.
I am not sure if we need to define this function, but it seems the shrink function also works for this purpose.

We can first clone the parent drawing area and then shrink it.

Alternatively, we can still use the split, but we can actually discard the one in the middle and use shrink to create the plotting area.

What I am planning to do is uses the drawing::area::shrink function if the sizes are negative to construct the label area.
I am not sure if we need to define this function, but it seems the shrink function also works for this purpose.

We can first clone the parent drawing area and then shrink it.

Feel free to copy, change any part. Its important for me that it do the trick.

Alternatively, we can still use the split, but we can actually discard the one in the middle and use shrink to create the plotting area.

As i mentoined earlier negative size is useful itself without labels being inset.

38 · 2019-10-11T02:37:17Z

src/chart/context.rs

            if orientation.0 == 0 {
                x0 = axis_range.start;
                x1 = axis_range.end;
            } else {
                y0 = axis_range.start;
                y1 = axis_range.end;
            }
+
+            if area.is_inset() {


Just little bit worry about the logic coupling. Since the drawing area is originally designed not to include too many details about chart.
And later we may have different types of weird coordinate system, and so I am prefer this flag lives in the 2D chart context rather than the generic drawing area.

Since my design needs to be implemented at builder stage it seemed logical to set inset field in Area structure. It looks good for me. Pretty generic flag :)

38 · 2019-10-11T02:40:31Z

src/chart/context.rs

@@ -335,17 +342,25 @@ impl<'a, DB: DrawingBackend, X: Ranged, Y: Ranged> ChartContext<'a, DB, RangedCo
                };

                area.draw_text(&t, label_style, (text_x, text_y))?;
-
+                // TODO: need some adjustments here


Actually I am thinking about do the both thing:

provide an API for tick_size adjustment

supports negative tick_size
And I am not sure if we can unify the implementation for the inward one.

This part of code i was most confused here i need just mirror all Path Lines coordinates.
Make tw/th = 0 and 0 = tw/th. I faced with some problems when realized there is truncate function. Its good if you know how to make it better.

38 · 2019-10-11T02:41:43Z

src/drawing/area.rs

@@ -106,6 +106,7 @@ pub struct DrawingArea<DB: DrawingBackend, CT: CoordTranslate> {
    backend: Rc<RefCell<DB>>,
    rect: Rect,
    coord: CT,
+    inset: bool,


Like I said, it's a little bit coupling between the drawing and the chart context here.

Looks good for me, but you are master. Do as you wish.

38 · 2019-10-11T02:42:32Z

src/drawing/area.rs

        })
    }

+    /// Alter area bounds
+    pub fn alter_diff(self, diff_tl: (i32,i32), diff_rb: (i32,i32)) -> Self {


Could we just use shrink ? It looks a little bit redundant to me , but I may be wrong

Sure, show me how :)

38 · 2019-10-11T02:43:01Z

src/drawing/backend.rs

@@ -39,6 +39,8 @@ pub trait BackendStyle {
    fn stroke_width(&self) -> u32 {
        1
    }
+
+    fn title(&self) -> &str { "" }


And why there's a title here?

It seems the title is not actually used by any of the backend.

Ahhh... i have heavy customized Webgl backend, and when dealing with all Shders, Vertexes and Fragments you need to know what kind of lines you are drawing, sometimes you need to pull through additional color to make some other stuff more smooth. So just treat it like helpful stuff. I'm using it heavily.

38 · 2019-10-11T06:33:08Z

Hmm, thanks for explain this. If you don't mind I think I am going to tweak a little bit before I merge it.

Also, if it's possible, I am very interested in your WebGL backend and it may be really helpful to me.
I am just asking, it's totally fine if you don't think it's able to open source.

serzhiio · 2019-10-11T08:32:44Z

Hmm, thanks for explain this. If you don't mind I think I am going to tweak a little bit before I merge it.

Its totally up to you. Make it better please :)

Also, if it's possible, I am very interested in your WebGL backend and it may be really helpful to me.
I am just asking, it's totally fine if you don't think it's able to open source.

My WebGl2 mod grew up to whole standalone library. I have no plans making it public so far, but i will consider it in future. Nevertheless if you have any questions abiut just let me know.

serzhiio added 9 commits September 13, 2019 13:30

Merge pull request #1 from 38/master

76da9a0

update

Merge pull request #2 from 38/master

75447e4

update

Merge pull request #3 from 38/master

00c57c8

update

Merge pull request #4 from 38/master

3ae10fb

update

Merge pull request #5 from 38/master

bcce66e

update

Merge pull request #6 from 38/master

38c5283

update

Merge pull request #7 from 38/master

ecdf96c

update

Merge pull request #8 from 38/master

036bc76

update

inset settings, negative value for label_sizes (behave like CSS negat…

f946699

…ive margin), title for ShapeStyle (better backend drawing customizing), some typos

38 reviewed Oct 11, 2019

View reviewed changes

38 merged commit f946699 into plotters-rs:master Oct 12, 2019

38 mentioned this pull request Oct 12, 2019

Feature that allows axis inside the plot #33

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inset feature #50

Inset feature #50

serzhiio commented Oct 10, 2019

codecov bot commented Oct 10, 2019 •

edited

Loading

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 left a comment

38 Oct 11, 2019

38 Oct 11, 2019

serzhiio Oct 11, 2019 •

edited

Loading

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 Oct 11, 2019

38 Oct 11, 2019

serzhiio Oct 11, 2019

38 commented Oct 11, 2019

serzhiio commented Oct 11, 2019

Inset feature #50

Inset feature #50

Conversation

serzhiio commented Oct 10, 2019

codecov bot commented Oct 10, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serzhiio Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38 commented Oct 11, 2019

serzhiio commented Oct 11, 2019

codecov bot commented Oct 10, 2019 •

edited

Loading

serzhiio Oct 11, 2019 •

edited

Loading