Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Implement GlyphItemIter manually #161

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion Gir.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ generate = [
"Pango.FontsetSimple",
"Pango.Glyph",
"Pango.GlyphItem",
"Pango.GlyphItemIter",
"Pango.GlyphString",
"Pango.GlyphUnit",
"Pango.Gravity",
Expand Down Expand Up @@ -209,6 +208,11 @@ status = "generate"
name = "language"
const = true

[[object]]
name = "Pango.GlyphItemIter"
#need manual iterator implementation
status = "ignored"

[[object]]
name = "Pango.Layout"
status = "generate"
Expand Down
56 changes: 0 additions & 56 deletions src/auto/glyph_item_iter.rs

This file was deleted.

3 changes: 0 additions & 3 deletions src/auto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ pub use self::font_metrics::FontMetrics;
mod glyph_item;
pub use self::glyph_item::GlyphItem;

mod glyph_item_iter;
pub use self::glyph_item_iter::GlyphItemIter;

mod glyph_string;
pub use self::glyph_string::GlyphString;

Expand Down
207 changes: 207 additions & 0 deletions src/glyph_item_iter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
// Copyright 2019, The Gtk-rs Project Developers.
// See the COPYRIGHT file at the top-level directory of this distribution.
// Licensed under the MIT license, see the LICENSE file or <http://opensource.org/licenses/MIT>

use glib::translate::*;
use pango_sys;
use std::marker::PhantomData;
use GlyphItem;

//Note: This type not exported
glib_wrapper! {
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct GlyphItemIter(Boxed<pango_sys::PangoGlyphItemIter>);
Copy link
Member

Choose a reason for hiding this comment

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

This needs a lifetime parameter. The the glyph item used for initialization must stay alive at least as long as the iterator. The iterator "borrows" the item.


match fn {
copy => |ptr| pango_sys::pango_glyph_item_iter_copy(mut_override(ptr)),
free => |ptr| pango_sys::pango_glyph_item_iter_free(ptr),
init => |_ptr| (),
clear => |_ptr| (),
get_type => || pango_sys::pango_glyph_item_iter_get_type(),
}
}

impl GlyphItemIter {
pub(crate) unsafe fn init_end(glyph_item: &GlyphItem, text: &str) -> Option<GlyphItemIter> {
let mut iter = GlyphItemIter::uninitialized();
let ret = from_glib(pango_sys::pango_glyph_item_iter_init_end(
iter.to_glib_none_mut().0,
mut_override(glyph_item.to_glib_none().0),
text.to_glib_none().0,
));
if ret {
Some(iter)
} else {
None
}
}

pub(crate) unsafe fn init_start(glyph_item: &GlyphItem, text: &str) -> Option<GlyphItemIter> {
let mut iter = GlyphItemIter::uninitialized();
let ret = from_glib(pango_sys::pango_glyph_item_iter_init_start(
iter.to_glib_none_mut().0,
mut_override(glyph_item.to_glib_none().0),
text.to_glib_none().0,
));
if ret {
Some(iter)
} else {
None
}
}

pub fn next_cluster(&mut self) -> bool {
unsafe {
from_glib(pango_sys::pango_glyph_item_iter_next_cluster(
self.to_glib_none_mut().0,
))
}
}

pub fn prev_cluster(&mut self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe there should also be Iterator, DoubleEndedIterator impls for easier usage of this, that have a struct IterItem { start_index: i32, ...} as item?

Copy link
Member Author

Choose a reason for hiding this comment

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

IMHO this need be other type instead of this,
with lifetime etc.

Copy link
Member

Choose a reason for hiding this comment

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

Why? This literally is an iterator :)

A lifetime parameter is also needed on this type

Copy link
Member Author

Choose a reason for hiding this comment

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

Because reverse variant (from iter_end) next() need call prev_cluster

Copy link
Member Author

Choose a reason for hiding this comment

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

also first call next need return "current" value instead moving ( for reverse next_back need return that)

Copy link
Member

Choose a reason for hiding this comment

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

I see, so might indeed need an adapter struct :) Weird API!

unsafe {
from_glib(pango_sys::pango_glyph_item_iter_prev_cluster(
self.to_glib_none_mut().0,
))
}
}

pub fn to_data(&self) -> GlyphItemIteratorData {
GlyphItemIteratorData {
start_glyph: self.0.start_glyph as usize,
end_glyph: self.0.end_glyph as usize,
start_index: self.0.start_index as usize,
end_index: self.0.end_index as usize,
start_char: self.0.start_char as usize,
end_char: self.0.end_char as usize,
}
}
}

pub struct GlyphItemIteratorData {
pub start_glyph: usize,
pub start_index: usize,
pub start_char: usize,

pub end_glyph: usize,
pub end_index: usize,
pub end_char: usize,
}

enum NormalIterator {}
enum ReverseIterator {}

struct GlyphItemIterator<'a, T> {
item: &'a GlyphItem,
text: &'a str,
iter: Option<GlyphItemIter>,
_mode: PhantomData<T>,
}

impl GlyphItem {
pub fn iter<'a>(
&'a self,
text: &'a str,
) -> impl DoubleEndedIterator<Item = GlyphItemIteratorData> + 'a {
GlyphItemIterator::<NormalIterator> {
item: self,
text,
iter: None,
_mode: PhantomData,
}
}

pub fn reverse_iter<'a>(
&'a self,
text: &'a str,
Copy link
Member

Choose a reason for hiding this comment

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

How do we ensure that the text corresponds to the glyph item? From reading the code it seems like we otherwise would get invalid memory accesses (if the text is smaller than the glyph)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know way to ensure :(

Copy link
Member Author

Choose a reason for hiding this comment

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

We can get PangoItem.length and compare with text length and chars count, but it not give us 100%

Copy link
Member

Choose a reason for hiding this comment

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

I don't know either, needs someone to dig into this API

Copy link
Member

Choose a reason for hiding this comment

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

@EPashkin This one is the remaining issue? I guess this needs some talking to Pango developers to figure out what to do about this here :)

Copy link
Member Author

Choose a reason for hiding this comment

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

IMHO yes

Copy link
Member

Choose a reason for hiding this comment

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

Any news on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@GuillaumeGomez Sorry, but I still have no idea how do this safe,
so better just leave it for now.

Copy link
Member

Choose a reason for hiding this comment

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

👍

) -> impl DoubleEndedIterator<Item = GlyphItemIteratorData> + 'a {
GlyphItemIterator::<ReverseIterator> {
item: self,
text,
iter: None,
_mode: PhantomData,
}
}
}

impl<'a> Iterator for GlyphItemIterator<'a, NormalIterator> {
type Item = GlyphItemIteratorData;

fn next(&mut self) -> Option<Self::Item> {
if let Some(ref mut iter) = self.iter {
if iter.next_cluster() {
Some(iter.to_data())
} else {
None
}
} else {
let iter = unsafe { GlyphItemIter::init_start(self.item, self.text) };
if let Some(iter) = iter {
let data = iter.to_data();
self.iter = Some(iter);
Some(data)
} else {
None
}
}
}
}

impl<'a> Iterator for GlyphItemIterator<'a, ReverseIterator> {
type Item = GlyphItemIteratorData;

fn next(&mut self) -> Option<Self::Item> {
if let Some(ref mut iter) = self.iter {
if iter.prev_cluster() {
Some(iter.to_data())
} else {
None
}
} else {
let iter = unsafe { GlyphItemIter::init_end(self.item, self.text) };
if let Some(iter) = iter {
let data = iter.to_data();
self.iter = Some(iter);
Some(data)
} else {
None
}
}
}
}

impl<'a> DoubleEndedIterator for GlyphItemIterator<'a, NormalIterator> {
fn next_back(&mut self) -> Option<Self::Item> {
if let Some(ref mut iter) = self.iter {
if iter.prev_cluster() {
Some(iter.to_data())
} else {
None
}
} else if let Some(iter) = unsafe { GlyphItemIter::init_end(self.item, self.text) } {
let data = iter.to_data();
self.iter = Some(iter);
Some(data)
} else {
None
}
}
}

impl<'a> DoubleEndedIterator for GlyphItemIterator<'a, ReverseIterator> {
fn next_back(&mut self) -> Option<Self::Item> {
if let Some(ref mut iter) = self.iter {
if iter.next_cluster() {
Some(iter.to_data())
} else {
None
}
} else if let Some(iter) = unsafe { GlyphItemIter::init_start(self.item, self.text) } {
let data = iter.to_data();
self.iter = Some(iter);
Some(data)
} else {
None
}
}
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod attr_list;
pub mod attribute;
pub mod font_description;
mod functions;
mod glyph_item_iter;
pub mod gravity;
pub mod item;
pub mod language;
Expand Down