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

Background images enhancements, fix FB2 footnotes alignment #353

Merged
merged 8 commits into from
Jul 5, 2020
Merged

Background images enhancements, fix FB2 footnotes alignment #353

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f64d682
(Upstream) HyphMan cleanup
virxkane Jul 5, 2020
e68bfee
Rendering methods: remove erm_runin
poire-z Jul 5, 2020
373309d
Text formatting: simplify 'display: run-in' handling
poire-z Jul 5, 2020
10ac343
Simplify background image drawing
poire-z Jul 5, 2020
53cd73c
CSS: adds support for "background-size" property
poire-z Jul 5, 2020
56b5ec0
CSS: support background:url("data:image/png;base64,...)
poire-z Jul 5, 2020
fd0523b
Fix elements cancelling inherited "white-space: pre"
poire-z Jul 5, 2020
546b96d
initNodeStyle(): skip some possibly costly validation
poire-z Jul 5, 2020
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
16 changes: 5 additions & 11 deletions crengine/include/cssdef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,6 @@ enum css_background_repeat_value_t {
css_background_r_inherit,
css_background_r_none
};
enum css_background_attachment_value_t {
css_background_scroll,
css_background_fixed,
css_background_local,
css_background_a_initial,
css_background_a_inherit,
css_background_a_none
};
enum css_background_position_value_t {
css_background_left_top,
css_background_left_center,
Expand Down Expand Up @@ -330,9 +322,11 @@ enum css_direction_t {
};

enum css_generic_value_t {
css_generic_auto = -1, // (css_val_unspecified, css_generic_auto), for "margin: auto"
css_generic_normal = -2, // (css_val_unspecified, css_generic_normal), for "line-height: normal"
css_generic_transparent = -3 // (css_val_unspecified, css_generic_transparent), for "color: transparent"
css_generic_auto = -1, // (css_val_unspecified, css_generic_auto), for "margin: auto"
css_generic_normal = -2, // (css_val_unspecified, css_generic_normal), for "line-height: normal"
css_generic_transparent = -3, // (css_val_unspecified, css_generic_transparent), for "color: transparent"
css_generic_contain = -4, // (css_val_unspecified, css_generic_contain), for "background-size: contain"
css_generic_cover = -5 // (css_val_unspecified, css_generic_cover), for "background-size: cover"
};

// Non standard property for providing hints to crengine via style tweaks
Expand Down
34 changes: 18 additions & 16 deletions crengine/include/lvstyles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,19 @@ enum css_style_rec_important_bit {
imp_bit_border_color_left = 1ULL << 45,
imp_bit_background_image = 1ULL << 46,
imp_bit_background_repeat = 1ULL << 47,
imp_bit_background_attachment = 1ULL << 48,
imp_bit_background_position = 1ULL << 49,
imp_bit_border_collapse = 1ULL << 50,
imp_bit_border_spacing_h = 1ULL << 51,
imp_bit_border_spacing_v = 1ULL << 52,
imp_bit_orphans = 1ULL << 53,
imp_bit_widows = 1ULL << 54,
imp_bit_float = 1ULL << 55,
imp_bit_clear = 1ULL << 56,
imp_bit_direction = 1ULL << 57,
imp_bit_content = 1ULL << 58,
imp_bit_cr_hint = 1ULL << 59
imp_bit_background_position = 1ULL << 48,
imp_bit_background_size_h = 1ULL << 49,
imp_bit_background_size_v = 1ULL << 50,
imp_bit_border_collapse = 1ULL << 51,
imp_bit_border_spacing_h = 1ULL << 52,
imp_bit_border_spacing_v = 1ULL << 53,
imp_bit_orphans = 1ULL << 54,
imp_bit_widows = 1ULL << 55,
imp_bit_float = 1ULL << 56,
imp_bit_clear = 1ULL << 57,
imp_bit_direction = 1ULL << 58,
imp_bit_content = 1ULL << 59,
imp_bit_cr_hint = 1ULL << 60
};

// Style handling flags
Expand All @@ -101,8 +102,8 @@ struct css_style_rec_tag {
int refCount; // for reference counting
lUInt32 hash; // cache calculated hash value here
lUInt64 important; // bitmap for !important (used only by LVCssDeclaration)
// we have currently below 60 css properties
// lvstsheet knows about 82, which are mapped to these 60
// we have currently below 61 css properties
// lvstsheet knows about 83, which are mapped to these 61
// update bits above if you add new properties below
lUInt64 importance; // bitmap for important bit's importance/origin
// (allows for 2 level of !important importance)
Expand Down Expand Up @@ -142,8 +143,8 @@ struct css_style_rec_tag {
css_length_t border_color[4]; ///< border-top-color, -right-, -bottom-, -left-
lString8 background_image;
css_background_repeat_value_t background_repeat;
css_background_attachment_value_t background_attachment;
css_background_position_value_t background_position;
css_length_t background_size[2];//first width and second height
css_border_collapse_value_t border_collapse;
css_length_t border_spacing[2];//first horizontal and the second vertical spacing
css_orphans_widows_value_t orphans;
Expand Down Expand Up @@ -194,7 +195,6 @@ struct css_style_rec_tag {
, border_style_right(css_border_none)
, border_style_left(css_border_none)
, background_repeat(css_background_r_none)
, background_attachment(css_background_a_none)
, background_position(css_background_p_none)
, border_collapse(css_border_seperate)
, orphans(css_orphans_widows_inherit)
Expand All @@ -215,6 +215,8 @@ struct css_style_rec_tag {
border_width[1] = css_length_t(css_val_unspecified, 0);
border_width[2] = css_length_t(css_val_unspecified, 0);
border_width[3] = css_length_t(css_val_unspecified, 0);
background_size[0] = css_length_t(css_val_unspecified, 0);
background_size[1] = css_length_t(css_val_unspecified, 0);
}
void AddRef() { refCount++; }
int Release() { return --refCount; }
Expand Down
74 changes: 73 additions & 1 deletion crengine/src/lvrend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3480,8 +3480,9 @@ void copystyle( css_style_ref_t source, css_style_ref_t dest )
dest->border_color[3]=source->border_color[3];
dest->background_image=source->background_image;
dest->background_repeat=source->background_repeat;
dest->background_attachment=source->background_attachment;
dest->background_position=source->background_position;
dest->background_size[0]=source->background_size[0];
dest->background_size[1]=source->background_size[1];
dest->border_collapse=source->border_collapse;
dest->border_spacing[0]=source->border_spacing[0];
dest->border_spacing[1]=source->border_spacing[1];
Expand Down Expand Up @@ -7840,8 +7841,79 @@ void DrawBackgroundImage(ldomNode *enode,LVDrawBuf & drawbuf,int x0,int y0,int d
lString16 filepath = lString16(style->background_image.c_str());
LVImageSourceRef img = enode->getParentNode()->getDocument()->getObjectImageSource(filepath);
if (!img.isNull()) {
// Native image size
int img_w =img->GetWidth();
int img_h =img->GetHeight();

// See if background-size specified and we need to adjust image native size
css_length_t bg_w = style->background_size[0];
css_length_t bg_h = style->background_size[1];
if ( bg_w.type != css_val_unspecified || bg_w.value != 0 || bg_h.type != css_val_unspecified || bg_h.value != 0 ) {
int new_w = 0;
int new_h = 0;
RenderRectAccessor fmt( enode );
int container_w = fmt.getWidth();
int container_h = fmt.getHeight();
bool check_lengths = true;
if ( bg_w.type == css_val_unspecified && bg_h.type == css_val_unspecified ) {
if ( bg_w.value == css_generic_contain && bg_h.value == css_generic_contain ) {
// Image should be fully contained in container (no crop)
int scale_w = 1024 * container_w / img_w;
Copy link
Member

Choose a reason for hiding this comment

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

Why 1024?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's reverted below when applied.
This is just to avoid int scale_w = 300 / 400 to be int 0 with int arithmetic.
Dunno if there's a proper way to do that :) @NiLuJe ?

Copy link
Member

@NiLuJe NiLuJe Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Short of storing the scale as a proper float (which would imply also casting one of the DIV operands to float), not really.

See also https://en.wikipedia.org/wiki/Fixed-point_arithmetic & http://www.sunshine2k.de/articles/coding/fp/sunfp.html for more complex trickery (e.g., 16.16 is how FreeType does arithmetics).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I guess that we'll probably never have images or container larger than 65536px, so I guess I could use 65536 = 2^16 as the multiplier (instead of 1024 = 2^10), and we'll fit in a 32-bits int. Right ?

int scale_w = (container_w<<16) / img_w
[...]
new_h = (img_h * scale_w) >> 16;

Or there's some risk I don't see?

Copy link
Member

@NiLuJe NiLuJe Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I mean, unless it's a really hot code path, I'd probably jump the gun and use a float, nothing we run on lacks an FPU.

i.e., float scale_w = container_w / (float) img_w;.

When going the other way around (i.e. int smthg = scale * int_somthg), the implicit cast will truncate by default, so you may need to ifloorf/iceilf/iroundf (which are C99 standard, and/or GCC/Clang builtins that return an int instead of a float).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, given that with rgrep float crengine/, I only see my stuff for supporting CSS floats, I'd say crengine is fully fixed-point arithmetic ! So, not going to introduce the first float variable :)
Ok, so it will be:

unsigned int scale_w = (container_w<<16) / img_w
[...]
new_h = (img_h * scale_w) >> 16;

Copy link
Member

@NiLuJe NiLuJe Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

See the overflow warnings above w/ 65536 ;).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so, let's say the max image or contained width or height we'll meet will be 65535px :)

echo $(( (2 ** 16) * (2**15) ))
2147483648

or should I use <<15 or <<14 to be safer ?

Copy link
Contributor Author

@poire-z poire-z Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Oh my, but in new_h = (img_h * scale_w) >> 16 , (img_h * scale_w) will overflow.
So, I guess it should be img_h * (scale_w >> 16) - but then I can't figure if it's useful or if the >>16 serves nothing :/

Copy link
Contributor Author

@poire-z poire-z Jul 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

OK, letting the 1024 as-is. I'll rethink that when I have to - or never :)
And letting it as signed int - dunno if in some edge cases, I can have negative container widths...

int scale_h = 1024 * container_h / img_h;
if ( scale_w < scale_h ) {
new_w = container_w;
new_h = img_h * scale_w / 1024;
}
else {
new_h = container_h;
new_w = img_w * scale_h / 1024;
}
check_lengths = false;
}
else if ( bg_w.value == css_generic_cover && bg_h.value == css_generic_cover ) {
// Image should fully cover container (crop allowed)
int scale_w = 1024 * container_w / img_w;
int scale_h = 1024 * container_h / img_h;
if ( scale_w > scale_h ) {
new_w = container_w;
new_h = img_h * scale_w / 1024;
}
else {
new_h = container_h;
new_w = img_w * scale_h / 1024;
}
check_lengths = false;
}
}
if ( check_lengths ) {
int em = enode->getFont()->getSize();
// These will compute to 0 if (css_val_unspecified, 0) when really not specified
new_w = lengthToPx(style->background_size[0], container_w, em);
new_h = lengthToPx(style->background_size[1], container_h, em);
if ( new_w == 0 ) {
if ( new_h == 0 ) { // keep image native size
new_h = img_h;
new_w = img_w;
}
else { // use style height, keep aspect ratio
new_w = img_w * new_h / img_h;
}
}
else if ( new_h == 0 ) { // use style width, keep aspect ratio
new_h = new_w * img_h / img_w;
}
}
if ( new_w == 0 || new_h == 0 ) {
// width or height computed to 0: nothing to draw
return;
}
if ( new_w != img_w || new_h != img_h ) {
img = LVCreateStretchFilledTransform(img, new_w, new_h, IMG_TRANSFORM_STRETCH, IMG_TRANSFORM_STRETCH, 0, 0);
img_w = new_w;
img_h = new_h;
}
}

// We can use some crengine facilities for background repetition and position,
// which has the advantage that img will be decoded once even if tiling it many
// times and if the target is many screen-heights long (like <BODY> could be).
Expand Down
57 changes: 48 additions & 9 deletions crengine/src/lvstsheet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ enum css_decl_code {
cssd_background,
cssd_background_image,
cssd_background_repeat,
cssd_background_attachment,
cssd_background_position,
cssd_background_size,
cssd_border_collapse,
cssd_border_spacing,
cssd_orphans,
Expand Down Expand Up @@ -197,8 +197,8 @@ static const char * css_decl_name[] = {
"background",
"background-image",
"background-repeat",
"background-attachment",
"background-position",
"background-size",
"border-collapse",
"border-spacing",
"orphans",
Expand Down Expand Up @@ -393,6 +393,7 @@ static bool parse_number_value( const char * & str, css_length_t & value,
bool accept_negative=false,
bool accept_auto=false,
bool accept_normal=false,
bool accept_contain_cover=false,
bool is_font_size=false )
{
const char * orig_pos = str;
Expand All @@ -414,6 +415,18 @@ static bool parse_number_value( const char * & str, css_length_t & value,
value.value = css_generic_normal;
return true;
}
if ( accept_contain_cover ) {
if ( substr_icompare( "contain", str ) ) {
value.type = css_val_unspecified;
value.value = css_generic_contain;
return true;
}
if ( substr_icompare( "cover", str ) ) {
value.type = css_val_unspecified;
value.value = css_generic_cover;
return true;
}
}
if ( is_font_size ) {
// Absolute-size keywords, based on the default font size (which is medium)
// Factors as suggested in https://drafts.csswg.org/css-fonts-3/#absolute-size-value
Expand Down Expand Up @@ -2253,7 +2266,7 @@ bool LVCssDeclaration::parse( const char * &decl, bool higher_importance, lxmlDo
if ( prop_code==cssd_font_size )
is_font_size = true;
css_length_t len;
if ( parse_number_value( decl, len, accept_percent, accept_negative, accept_auto, accept_normal, is_font_size) ) {
if ( parse_number_value( decl, len, accept_percent, accept_negative, accept_auto, accept_normal, false, is_font_size) ) {
buf<<(lUInt32) (prop_code | importance | parse_important(decl));
buf<<(lUInt32) len.type;
buf<<(lUInt32) len.value;
Expand Down Expand Up @@ -2598,9 +2611,6 @@ bool LVCssDeclaration::parse( const char * &decl, bool higher_importance, lxmlDo
else if ( n==24 ) n=0; // "inherit" = "left top"
}
break;
case cssd_background_attachment:
n = parse_name( decl, css_bg_attachment_names, -1 );
break;
case cssd_background:
{
// Limited parsing of this possibly complex property
Expand Down Expand Up @@ -2681,6 +2691,34 @@ bool LVCssDeclaration::parse( const char * &decl, bool higher_importance, lxmlDo
}
}
break;
case cssd_background_size:
{
// https://developer.mozilla.org/en-US/docs/Web/CSS/background-size
css_length_t len[2];
int i;
for (i = 0; i < 2; i++) {
if ( !parse_number_value( decl, len[i], true, false, true, false, true ) )
break;
}
if (i) {
if (i == 1) { // Only 1 value parsed
if ( len[0].type == css_val_unspecified ) { // "auto", "contain" or "cover"
len[1].type = css_val_unspecified;
len[1].value = len[0].value;
}
else { // first value is a length: second value should be "auto"
len[1].type = css_val_unspecified;
len[1].value = css_generic_auto;
}
}
buf<<(lUInt32) (prop_code | importance | parse_important(decl));
for (i = 0; i < 2; i++) {
buf<<(lUInt32) len[i].type;
buf<<(lUInt32) len[i].value;
}
}
}
break;
case cssd_border_spacing:
{
css_length_t len[2];
Expand Down Expand Up @@ -2992,12 +3030,13 @@ void LVCssDeclaration::apply( css_style_rec_t * style )
case cssd_background_repeat:
style->Apply( (css_background_repeat_value_t) *p++, &style->background_repeat, imp_bit_background_repeat, is_important );
break;
case cssd_background_attachment:
style->Apply( (css_background_attachment_value_t) *p++, &style->background_attachment, imp_bit_background_attachment, is_important );
break;
case cssd_background_position:
style->Apply( (css_background_position_value_t) *p++, &style->background_position, imp_bit_background_position, is_important );
break;
case cssd_background_size:
style->Apply( read_length(p), &style->background_size[0], imp_bit_background_size_h, is_important );
style->Apply( read_length(p), &style->background_size[1], imp_bit_background_size_v, is_important );
break;
case cssd_border_spacing:
style->Apply( read_length(p), &style->border_spacing[0], imp_bit_border_spacing_h, is_important );
style->Apply( read_length(p), &style->border_spacing[1], imp_bit_border_spacing_v, is_important );
Expand Down
14 changes: 9 additions & 5 deletions crengine/src/lvstyles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ lUInt32 calcHash(font_ref_t & f)
lUInt32 calcHash(css_style_rec_t & rec)
{
if ( !rec.hash )
rec.hash = (((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
rec.hash = ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
(lUInt32)(rec.important >> 32)) * 31
+ (lUInt32)(rec.important & 0xFFFFFFFFULL)) * 31
+ (lUInt32)(rec.importance >> 32)) * 31
Expand Down Expand Up @@ -93,8 +93,9 @@ lUInt32 calcHash(css_style_rec_t & rec)
+ (lUInt32)rec.border_color[2].pack()) * 31
+ (lUInt32)rec.border_color[3].pack()) * 31
+ (lUInt32)rec.background_repeat)*31
+ (lUInt32)rec.background_attachment)*31
+ (lUInt32)rec.background_position)*31
+ (lUInt32)rec.background_size[0].pack())*31
+ (lUInt32)rec.background_size[1].pack())*31
+ (lUInt32)rec.font_family) * 31
+ (lUInt32)rec.border_collapse)*31
+ (lUInt32)rec.border_spacing[0].pack())*31
Expand Down Expand Up @@ -161,8 +162,9 @@ bool operator == (const css_style_rec_t & r1, const css_style_rec_t & r2)
r1.border_color[3]==r2.border_color[3]&&
r1.background_image==r2.background_image&&
r1.background_repeat==r2.background_repeat&&
r1.background_attachment==r2.background_attachment&&
r1.background_position==r2.background_position&&
r1.background_size[0]==r2.background_size[0]&&
r1.background_size[1]==r2.background_size[1]&&
r1.border_collapse==r2.border_collapse&&
r1.border_spacing[0]==r2.border_spacing[0]&&
r1.border_spacing[1]==r2.border_spacing[1]&&
Expand Down Expand Up @@ -349,8 +351,9 @@ bool css_style_rec_t::serialize( SerialBuf & buf )
ST_PUT_LEN4(border_color);
buf<<background_image;
ST_PUT_ENUM(background_repeat);
ST_PUT_ENUM(background_attachment);
ST_PUT_ENUM(background_position);
ST_PUT_LEN(background_size[0]);
ST_PUT_LEN(background_size[1]);
ST_PUT_ENUM(border_collapse);
ST_PUT_LEN(border_spacing[0]);
ST_PUT_LEN(border_spacing[1]);
Expand Down Expand Up @@ -409,8 +412,9 @@ bool css_style_rec_t::deserialize( SerialBuf & buf )
ST_GET_LEN4(border_color);
buf>>background_image;
ST_GET_ENUM(css_background_repeat_value_t ,background_repeat);
ST_GET_ENUM(css_background_attachment_value_t ,background_attachment);
ST_GET_ENUM(css_background_position_value_t ,background_position);
ST_GET_LEN(background_size[0]);
ST_GET_LEN(background_size[1]);
ST_GET_ENUM(css_border_collapse_value_t ,border_collapse);
ST_GET_LEN(border_spacing[0]);
ST_GET_LEN(border_spacing[1]);
Expand Down