lvrend: fix positioning of bottom border on empty block elements

Fix some possible issues with empty block elements having some bottom border (included in padding_bottom) when previous vertical margins are collapsing. (Not 100% certain this is the right fix that won't cause side effects...)
koreader · Apr 6, 2024 · f4370bc · f4370bc
1 parent d469daa
commit f4370bc
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 10 deletions.
diff --git a/crengine/src/lvrend.cpp b/crengine/src/lvrend.cpp
@@ -5894,11 +5894,7 @@ class FlowState {
         last_split_after_flag = RN_GET_SPLIT_AFTER(flags);
     }
 
-    int addContentLine( int height, int flags, int baseline=NO_BASELINE_UPDATE, bool is_padding=false ) {
-        // As we may push vertical margins, we return the total height moved
-        // (needed when adding bottom padding that may push inner vertical
-        // margins, which should be accounted in the element height).
-        int start_c_y = c_y;
+    void addContentLine( int height, int flags, int baseline=NO_BASELINE_UPDATE, bool is_padding=false ) {
         int line_dir_flag = direction == REND_DIRECTION_RTL ? RN_LINE_IS_RTL : 0;
         // Ensure avoid_pb_inside
         if ( avoid_pb_inside_just_toggled_off ) {
@@ -5976,7 +5972,6 @@ class FlowState {
                 }
             }
         }
-        return c_y - start_c_y;
     }
 
     void addContentSpace( int height, int line_h, bool split_avoid_before,
@@ -8560,9 +8555,24 @@ void renderBlockElementEnhanced( FlowState * flow, ldomNode * enode, int x, int
                 // (Firefox, with a float taller than text, both in another
                 // float, applies bottom padding after the inner float)
                 if (padding_bottom>0) {
-                    int padding_bottom_with_inner_pushed_vm = flow->addContentLine(padding_bottom, RN_SPLIT_BEFORE_AVOID, 0, true);
-                    h += padding_bottom_with_inner_pushed_vm;
-                    bottom_overflow -= padding_bottom_with_inner_pushed_vm;
+                    // We may push any inner vertical margin: gather how much we moved
+                    int c_y = flow->getCurrentAbsoluteY();
+                    flow->addContentLine(padding_bottom, RN_SPLIT_BEFORE_AVOID, 0, true);
+                    int padding_bottom_with_inner_pushed_vm = flow->getCurrentAbsoluteY() - c_y;
+                    if (h <= 0) {
+                        // Empty block: any pushed vertical margin can be put outside this block
+                        // Note: this different behaviour seems needed for this bottom padding/border
+                        // to be drawn at the expected position. Not really sure what happens, it
+                        // might be that pushVerticalMargin() shifts or not our node differently if
+                        // it happens to have no content and didn't call other flow methods...
+                        h += padding_bottom;
+                    }
+                    else {
+                        // We have some content/height: any pushed vertical margin
+                        // is inside and part of our block height
+                        h += padding_bottom_with_inner_pushed_vm;
+                        bottom_overflow -= padding_bottom_with_inner_pushed_vm;
+                    }
                 }
 
                 if (h <=0) {

diff --git a/crengine/src/lvtinydom.cpp b/crengine/src/lvtinydom.cpp
@@ -95,7 +95,7 @@ extern const int gDOMVersionCurrent = DOM_VERSION_CURRENT;
 // increment to force complete reload/reparsing of old file
 #define CACHE_FILE_FORMAT_VERSION "3.05.74k"
 /// increment following value to force re-formatting of old book after load
-#define FORMATTING_VERSION_ID 0x0032
+#define FORMATTING_VERSION_ID 0x0033
 
 #ifndef DOC_DATA_COMPRESSION_LEVEL
 /// data compression level (0=no compression, 1=fast compressions, 3=normal compression)