Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Inset feature #50

Merged
merged 9 commits into from
Oct 12, 2019
Merged

Inset feature #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
76da9a0
Merge pull request #1 from 38/master
serzhiio Sep 13, 2019
75447e4
Merge pull request #2 from 38/master
serzhiio Sep 23, 2019
00c57c8
Merge pull request #3 from 38/master
serzhiio Sep 24, 2019
3ae10fb
Merge pull request #4 from 38/master
serzhiio Sep 30, 2019
bcce66e
Merge pull request #5 from 38/master
serzhiio Sep 30, 2019
38c5283
Merge pull request #6 from 38/master
serzhiio Oct 1, 2019
ecdf96c
Merge pull request #7 from 38/master
serzhiio Oct 5, 2019
036bc76
Merge pull request #8 from 38/master
serzhiio Oct 9, 2019
f946699
inset settings, negative value for label_sizes (behave like CSS negat…
serzhiio Oct 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 45 additions & 11 deletions src/chart/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

root_area: &'a DrawingArea<DB, Shift>,
title: Option<(String, TextStyle<'b>)>,
margin: [u32; 4],
Expand All @@ -31,6 +32,7 @@ impl<'a, 'b, DB: DrawingBackend> ChartBuilder<'a, 'b, DB> {
pub fn on(root: &'a DrawingArea<DB, Shift>) -> Self {
Self {
label_area_size: [0; 4],
label_area_inset: [false; 4],
root_area: root,
title: None,
margin: [0; 4],
Expand Down Expand Up @@ -74,36 +76,56 @@ impl<'a, 'b, DB: DrawingBackend> ChartBuilder<'a, 'b, DB> {

/// Set the size of X label area
/// - `size`: The height of the x label area, if x is 0, the chart doesn't have the X label area
pub fn x_label_area_size(&mut self, size: u32) -> &mut Self {
pub fn x_label_area_size(&mut self, size: i32) -> &mut Self {
self.label_area_size[1] = size;
self
}

pub fn inset_x_labels(&mut self) -> &mut Self {
self.label_area_inset[1] = true;
self
}

/// Set the size of the Y label area
/// - `size`: The width of the Y label area. If size is 0, the chart doesn't have Y label area
pub fn y_label_area_size(&mut self, size: u32) -> &mut Self {
pub fn y_label_area_size(&mut self, size: i32) -> &mut Self {
self.label_area_size[2] = size;
self
}

pub fn inset_y_labels(&mut self) -> &mut Self {
self.label_area_inset[2] = true;
self
}

/// Set the size of X label area on the top of the chart
/// - `size`: The height of the x label area, if x is 0, the chart doesn't have the X label area
pub fn top_x_label_area_size(&mut self, size: u32) -> &mut Self {
pub fn top_x_label_area_size(&mut self, size: i32) -> &mut Self {
self.label_area_size[0] = size;
self
}

pub fn inset_top_x_labels(&mut self) -> &mut Self {
self.label_area_inset[0] = true;
self
}

/// Set the size of the Y label area on the right side
/// - `size`: The width of the Y label area. If size is 0, the chart doesn't have Y label area
pub fn right_y_label_area_size(&mut self, size: u32) -> &mut Self {
pub fn right_y_label_area_size(&mut self, size: i32) -> &mut Self {
self.label_area_size[3] = size;
self
}

pub fn inset_right_y_labels(&mut self) -> &mut Self {
self.label_area_inset[3] = true;
self
}

/// Set a label area size
/// - `pos`: THe position where the label area locted
/// - `size`: The size of the label area size
pub fn set_label_area_size(&mut self, pos: LabelAreaPosition, size: u32) -> &mut Self {
pub fn set_label_area_size(&mut self, pos: LabelAreaPosition, size: i32) -> &mut Self {
self.label_area_size[pos as usize] = size;
self
}
Expand Down Expand Up @@ -157,10 +179,9 @@ impl<'a, 'b, DB: DrawingBackend> ChartBuilder<'a, 'b, DB> {
let mut actual_drawing_area_pos = [0, h as i32, 0, w as i32];

for (idx, (dx, dy)) in (0..4).map(|idx| (idx, [(0, -1), (0, 1), (-1, 0), (1, 0)][idx])) {
let size = self.label_area_size[idx] as i32;

let split_point = if dx + dy < 0 { size } else { -size };

//let size = if self.label_area_size[idx] <= 0 { 0 } else { self.label_area_size[idx] };
let size = self.label_area_size[idx];
let split_point = if !self.label_area_inset[idx] { if dx + dy < 0 { size } else { -size } } else { 0 };
actual_drawing_area_pos[idx] += split_point;
}

Expand All @@ -180,7 +201,20 @@ impl<'a, 'b, DB: DrawingBackend> ChartBuilder<'a, 'b, DB> {
}
}

std::mem::swap(&mut drawing_area, splitted[4].as_mut().unwrap());
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@serzhiio serzhiio Oct 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

0 => area.clone().alter_new((None,None),(None,Some(*size))),
2 => area.clone().alter_new((None,None),(Some(*size),None)),
1 => area.clone().alter_new((None,Some(h as i32 - *size)),(None,Some(h as i32 - *size))),
3 => area.clone().alter_new((Some(w as i32 - *size),None),(None,None)),
_ => unreachable!(),
}.make_inset();
std::mem::swap(&mut label_areas[id], &mut Some(new_area));
}

std::mem::swap(&mut drawing_area, &mut splitted[4].as_mut().unwrap());

let mut pixel_range = drawing_area.get_pixel_range();
pixel_range.1 = pixel_range.1.end..pixel_range.1.start;
Expand Down
37 changes: 26 additions & 11 deletions src/chart/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,25 @@ impl<'a, DB: DrawingBackend, X: Ranged, Y: Ranged> ChartContext<'a, DB, RangedCo
}

if let Some(style) = axis_style {
let mut x0 = if orientation.0 > 0 { 0 } else { tw as i32 };
let mut y0 = if orientation.1 > 0 { 0 } else { th as i32 };
let mut x0 = if orientation.0 > 0 { 0 } else { tw as i32 };
let mut y0 = if orientation.1 > 0 { 0 } else { th as i32 };
let mut x1 = if orientation.0 >= 0 { 0 } else { tw as i32 };
let mut y1 = if orientation.1 >= 0 { 0 } else { th as i32 };


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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

// FIXME: looks like truncate func works strange with right side
if x0 == x1 { if x0 > 0 { x0=1; x1=1; } else { x0 = 10_000; x1 = 10_000; } };
if y0 == y1 { if y0 > 0 { y0=1; y1=1; } else { y0 = 10_000; y1 = 10_000; } };
}
area.draw(&Path::new(vec![(x0, y0), (x1, y1)], style.clone()))?;
}

Expand Down Expand Up @@ -313,20 +320,20 @@ impl<'a, DB: DrawingBackend, X: Ranged, Y: Ranged> ChartContext<'a, DB, RangedCo
.estimate_text_size(&t, &label_style.font)
.unwrap_or((0, 0));

// TODO: need some adjustments here for inset labels
let (cx, cy) = match orientation {
(dx, dy) if dx > 0 && dy == 0 => (right_most - w as i32, *p - y0),
(dx, dy) if dx < 0 && dy == 0 => (tw as i32 - label_dist - w as i32, *p - y0),
(dx, dy) if dx == 0 && dy > 0 => (*p - x0, label_dist + h as i32),
(dx, dy) if dx == 0 && dy < 0 => (*p - x0, th as i32 - label_dist - h as i32),
_ => panic!("Bug: Invlid orientation specification"),
_ => panic!("Bug: Invalid orientation specification"),
};

let should_draw = if orientation.0 == 0 {
cx >= 0 && cx + label_offset + w as i32 / 2 <= tw as i32
} else {
cy >= 0 && cy + label_offset + h as i32 / 2 <= th as i32
};

if should_draw {
let (text_x, text_y) = if orientation.0 == 0 {
(cx - w as i32 / 2 + label_offset, cy)
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I am thinking about do the both thing:

  1. provide an API for tick_size adjustment
  2. supports negative tick_size
    And I am not sure if we can unify the implementation for the inward one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if let Some(style) = axis_style {
let (kx0, ky0, kx1, ky1) = match orientation {
(dx, dy) if dx > 0 && dy == 0 => (0, *p - y0, tick_size, *p - y0),
(dx, dy) if dx < 0 && dy == 0 => {
// ---
(dx, dy) if dx > 0 && dy == 0 => if area.is_inset() {
(tw as i32 - tick_size, *p - y0, tw as i32, *p - y0)
}
(dx, dy) if dx == 0 && dy > 0 => (*p - x0, 0, *p - x0, tick_size),
(dx, dy) if dx == 0 && dy < 0 => {
} else{ (0, *p - y0, tick_size, *p - y0) },
// --- left labels
(dx, dy) if dx < 0 && dy == 0 => if area.is_inset() {
(0, *p - y0, tick_size, *p - y0)
} else {(tw as i32 - tick_size, *p - y0, tw as i32, *p - y0) },
// ---
(dx, dy) if dx == 0 && dy > 0 => if area.is_inset() {
(*p - x0, th as i32 - tick_size, *p - x0, th as i32)
}
} else { (*p - x0, 0, *p - x0, tick_size) },
// ---
(dx, dy) if dx == 0 && dy < 0 => if area.is_inset() {
(*p - x0, 0, *p - x0, tick_size)
} else { (*p - x0, th as i32 - tick_size, *p - x0, th as i32) },
_ => panic!("Bug: Invlid orientation specification"),
};
let line = Path::new(vec![(kx0, ky0), (kx1, ky1)], style.clone());
Expand Down
2 changes: 1 addition & 1 deletion src/coord/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl ReverseCoordTranslate for Shift {
}
}

/// We can compose an abitray transformation with a shift
/// We can compose an arbitrary transformation with a shift
pub struct ShiftAndTrans<T: CoordTranslate>(Shift, T);

impl<T: CoordTranslate> CoordTranslate for ShiftAndTrans<T> {
Expand Down
52 changes: 51 additions & 1 deletion src/drawing/area.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct Rect {
}

impl Rect {
/// Split the rectangle into a few smaller rectnagles
/// Split the rectangle into a few smaller rectangles
fn split<'a, BPI: IntoIterator<Item = &'a i32> + 'a>(
&'a self,
break_points: BPI,
Expand Down Expand Up @@ -106,6 +106,7 @@ pub struct DrawingArea<DB: DrawingBackend, CT: CoordTranslate> {
backend: Rc<RefCell<DB>>,
rect: Rect,
coord: CT,
inset: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

impl<DB: DrawingBackend, CT: CoordTranslate + Clone> Clone for DrawingArea<DB, CT> {
Expand All @@ -114,6 +115,7 @@ impl<DB: DrawingBackend, CT: CoordTranslate + Clone> Clone for DrawingArea<DB, C
backend: self.copy_backend_ref(),
rect: self.rect.clone(),
coord: self.coord.clone(),
inset: self.inset,
}
}
}
Expand Down Expand Up @@ -220,6 +222,7 @@ impl<DB: DrawingBackend, CT: CoordTranslate> DrawingArea<DB, CT> {
rect: self.rect.clone(),
backend: self.copy_backend_ref(),
coord: Shift((self.rect.x0, self.rect.y0)),
inset: self.inset,
}
}

Expand Down Expand Up @@ -331,6 +334,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
},
backend,
coord: Shift((0, 0)),
inset: false,
}
}

Expand All @@ -357,6 +361,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
rect: self.rect.clone(),
backend: self.copy_backend_ref(),
coord: coord_spec,
inset: self.inset,
}
}

Expand All @@ -371,6 +376,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
},
backend: self.copy_backend_ref(),
coord: Shift((self.rect.x0 + left, self.rect.y0 + top)),
inset: self.inset,
}
}

Expand All @@ -381,6 +387,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
rect: rect.clone(),
backend: self.copy_backend_ref(),
coord: Shift((rect.x0, rect.y0)),
inset: self.inset,
});

(ret.next().unwrap(), ret.next().unwrap())
Expand All @@ -393,6 +400,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
rect: rect.clone(),
backend: self.copy_backend_ref(),
coord: Shift((rect.x0, rect.y0)),
inset: self.inset,
});

(ret.next().unwrap(), ret.next().unwrap())
Expand All @@ -406,6 +414,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
rect: rect.clone(),
backend: self.copy_backend_ref(),
coord: Shift((rect.x0, rect.y0)),
inset: self.inset,
})
.collect()
}
Expand All @@ -422,6 +431,7 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
rect: rect.clone(),
backend: self.copy_backend_ref(),
coord: Shift((rect.x0, rect.y0)),
inset: self.inset,
})
.collect()
}
Expand Down Expand Up @@ -466,9 +476,49 @@ impl<DB: DrawingBackend> DrawingArea<DB, Shift> {
},
backend: self.copy_backend_ref(),
coord: Shift((self.rect.x0, self.rect.y0 + 10 + text_h as i32)),
inset: self.inset,
})
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, show me how :)

Self {
backend: self.backend,
rect: Rect{
x0: self.rect.x0 + diff_tl.0,
y0: self.rect.y0 + diff_tl.1,
x1: self.rect.x1 + diff_rb.0,
y1: self.rect.y1 + diff_rb.1,
},
coord: self.coord,
inset: self.inset,
}
}
/// Update area bounds
pub fn alter_new(self, tl: (Option<i32>,Option<i32>), rb: (Option<i32>,Option<i32>)) -> Self {
Self {
backend: self.backend,
rect: Rect{
x0: tl.0.unwrap_or(self.rect.x0),
y0: tl.1.unwrap_or(self.rect.y0),
x1: rb.0.unwrap_or(self.rect.x1),
y1: rb.1.unwrap_or(self.rect.y1),
},
coord: self.coord,
inset: self.inset,
}
}
/// Make area inset
pub fn make_inset(self) -> Self {
Self {
backend: self.backend,
rect: self.rect,
coord: self.coord,
inset: true,
}
}
/// Get area's inset value
pub fn is_inset(&self) -> bool { self.inset }
/// Draw text on the drawing area
pub fn draw_text(
&self,
Expand Down
3 changes: 3 additions & 0 deletions src/drawing/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ pub trait BackendStyle {
fn stroke_width(&self) -> u32 {
1
}

fn title(&self) -> &str { "" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why there's a title here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

impl<T: Color> BackendStyle for T {
Expand All @@ -56,6 +58,7 @@ impl BackendStyle for ShapeStyle {
fn stroke_width(&self) -> u32 {
self.stroke_width
}
fn title(&self) -> &'static str { self.title }
}

/// The drawing backend trait, which implemenets the low-level drawing APIs.
Expand Down
Loading