From a258d6507a9d4c7ec7bd951fd51d16e67fc2b561 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 5 Feb 2019 06:43:33 +0100 Subject: mmap all the font files! Font now uses the same in-memory format as the font files we have on disk. This allows us to simply mmap() the font files and not use any additional memory for them. Very cool! :^) Hacking on this exposed a bug in file-backed VMObjects where the first client to instantiate a VMObject for a specific inode also got to decide its size. Since file-backed VMObjects always have the same size as the underlying file, this made no sense, so I removed the ability to even set a size in that case. --- FontEditor/FontEditor.cpp | 15 +++--- Kernel/MemoryManager.cpp | 31 ++++++------- Kernel/MemoryManager.h | 4 +- Kernel/Process.cpp | 2 +- Kernel/Process.h | 1 + Kernel/i386.h | 10 ++++ LibGUI/GTextBox.cpp | 2 +- SharedGraphics/Font.cpp | 111 +++++++++++++++------------------------------ SharedGraphics/Font.h | 43 +++++++++++++++--- SharedGraphics/Painter.cpp | 23 ++++++++++ SharedGraphics/Painter.h | 2 + 11 files changed, 134 insertions(+), 110 deletions(-) diff --git a/FontEditor/FontEditor.cpp b/FontEditor/FontEditor.cpp index a4f073c000..879fabb56b 100644 --- a/FontEditor/FontEditor.cpp +++ b/FontEditor/FontEditor.cpp @@ -8,7 +8,9 @@ FontEditorWidget::FontEditorWidget(GWidget* parent) : GWidget(parent) { m_edited_font = Font::load_from_file("/saved.font"); - if (!m_edited_font) + if (m_edited_font) + m_edited_font = m_edited_font->clone(); + else m_edited_font = Font::default_font().clone(); m_glyph_map_widget = new GlyphMapWidget(*m_edited_font, this); @@ -181,7 +183,7 @@ void GlyphEditorWidget::paint_event(GPaintEvent&) painter.translate(1, 1); - auto& bitmap = font().glyph_bitmap(m_glyph); + auto bitmap = font().glyph_bitmap(m_glyph); for (int y = 0; y < font().glyph_height(); ++y) { for (int x = 0; x < font().glyph_width(); ++x) { @@ -214,17 +216,14 @@ void GlyphEditorWidget::draw_at_mouse(const GMouseEvent& event) bool unset = event.buttons() & GMouseButton::Right; if (!(set ^ unset)) return; - byte new_bit = set ? '#' : ' '; int x = (event.x() - 1) / m_scale; int y = (event.y() - 1) / m_scale; - auto& bitmap = font().glyph_bitmap(m_glyph); - auto* mutable_bits = const_cast(bitmap.bits()); + auto bitmap = font().glyph_bitmap(m_glyph); ASSERT((unsigned)x < bitmap.width()); ASSERT((unsigned)y < bitmap.height()); - auto& bit = mutable_bits[y * bitmap.width() + x]; - if (bit == new_bit) + if (bitmap.bit_at(x, y) == set) return; - bit = new_bit; + bitmap.set_bit_at(x, y, set); if (on_glyph_altered) on_glyph_altered(); update(); diff --git a/Kernel/MemoryManager.cpp b/Kernel/MemoryManager.cpp index 81b8842087..ad087917a1 100644 --- a/Kernel/MemoryManager.cpp +++ b/Kernel/MemoryManager.cpp @@ -304,15 +304,12 @@ bool MemoryManager::page_in_from_inode(Region& region, unsigned page_index_in_re auto& vmo_page = vmo.physical_pages()[region.first_page_index() + page_index_in_region]; - bool interrupts_were_enabled = are_interrupts_enabled(); - - if (!interrupts_were_enabled) - sti(); + // FIXME: Maybe this should have a separate class like InterruptFlagSaver? + InterruptDisabler disabler; + disabler.temporarily_sti(); LOCKER(vmo.m_paging_lock); - - if (!interrupts_were_enabled) - cli(); + disabler.temporarily_cli(); if (!vmo_page.is_null()) { kprintf("MM: page_in_from_inode() but page already present. Fine with me!\n"); @@ -321,9 +318,9 @@ bool MemoryManager::page_in_from_inode(Region& region, unsigned page_index_in_re } #ifdef MM_DEBUG - dbgprintf("MM: page_in_from_inode ready to read from inode, will write to L%x!\n", dest_ptr); + dbgprintf("MM: page_in_from_inode ready to read from inode\n"); #endif - sti(); // Oh god here we go... + disabler.temporarily_sti(); byte page_buffer[PAGE_SIZE]; auto& inode = *vmo.inode(); auto nread = inode.read_bytes(vmo.inode_offset() + ((region.first_page_index() + page_index_in_region) * PAGE_SIZE), PAGE_SIZE, page_buffer, nullptr); @@ -335,7 +332,7 @@ bool MemoryManager::page_in_from_inode(Region& region, unsigned page_index_in_re // If we read less than a page, zero out the rest to avoid leaking uninitialized data. memset(page_buffer + nread, 0, PAGE_SIZE - nread); } - cli(); + disabler.temporarily_cli(); vmo_page = allocate_physical_page(ShouldZeroFill::No); if (vmo_page.is_null()) { kprintf("MM: page_in_from_inode was unable to allocate a physical page\n"); @@ -610,7 +607,7 @@ Region::Region(LinearAddress a, size_t s, String&& n, bool r, bool w, bool cow) Region::Region(LinearAddress a, size_t s, RetainPtr&& inode, String&& n, bool r, bool w) : m_laddr(a) , m_size(s) - , m_vmo(VMObject::create_file_backed(move(inode), s)) + , m_vmo(VMObject::create_file_backed(move(inode))) , m_name(move(n)) , m_readable(r) , m_writable(w) @@ -661,13 +658,12 @@ void PhysicalPage::return_to_freelist() #endif } -RetainPtr VMObject::create_file_backed(RetainPtr&& inode, size_t size) +RetainPtr VMObject::create_file_backed(RetainPtr&& inode) { InterruptDisabler disabler; if (inode->vmo()) return static_cast(inode->vmo()); - size = ceil_div(size, PAGE_SIZE) * PAGE_SIZE; - auto vmo = adopt(*new VMObject(move(inode), size)); + auto vmo = adopt(*new VMObject(move(inode))); vmo->inode()->set_vmo(vmo.ptr()); return vmo; } @@ -720,10 +716,11 @@ VMObject::VMObject(PhysicalAddress paddr, size_t size) } -VMObject::VMObject(RetainPtr&& inode, size_t size) - : m_size(size) - , m_inode(move(inode)) +VMObject::VMObject(RetainPtr&& inode) + : m_inode(move(inode)) { + ASSERT(m_inode); + m_size = ceil_div(m_inode->size(), PAGE_SIZE) * PAGE_SIZE; m_physical_pages.resize(page_count()); MM.register_vmo(*this); } diff --git a/Kernel/MemoryManager.h b/Kernel/MemoryManager.h index c2473efa76..098e27e938 100644 --- a/Kernel/MemoryManager.h +++ b/Kernel/MemoryManager.h @@ -79,7 +79,7 @@ private: class VMObject : public Retainable { friend class MemoryManager; public: - static RetainPtr create_file_backed(RetainPtr&&, size_t); + static RetainPtr create_file_backed(RetainPtr&&); static RetainPtr create_anonymous(size_t); static RetainPtr create_framebuffer_wrapper(PhysicalAddress, size_t); RetainPtr clone(); @@ -99,7 +99,7 @@ public: Vector>& physical_pages() { return m_physical_pages; } private: - VMObject(RetainPtr&&, size_t); + VMObject(RetainPtr&&); explicit VMObject(VMObject&); explicit VMObject(size_t); VMObject(PhysicalAddress, size_t); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index c609e36044..f7672988c7 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -325,7 +325,7 @@ int Process::do_exec(const String& path, Vector&& arguments, Vectorinode(), descriptor->metadata().size); + auto vmo = VMObject::create_file_backed(descriptor->inode()); vmo->set_name(descriptor->absolute_path()); RetainPtr region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copy_ref(), 0, "helper", true, false); diff --git a/Kernel/Process.h b/Kernel/Process.h index ca4d68d689..08c535d59d 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -52,6 +52,7 @@ class Process : public InlineLinkedListNode, public Weakable { friend class InlineLinkedListNode; friend class WSWindowManager; // FIXME: Make a better API for allocate_region(). friend class GraphicsBitmap; // FIXME: Make a better API for allocate_region(). + friend class Font; //FIXME: This is beyond gross. public: static Process* create_kernel_process(String&& name, void (*entry)()); static Process* create_user_process(const String& path, uid_t, gid_t, pid_t ppid, int& error, Vector&& arguments = Vector(), Vector&& environment = Vector(), TTY* = nullptr); diff --git a/Kernel/i386.h b/Kernel/i386.h index ad25551f41..23a84e0ab9 100644 --- a/Kernel/i386.h +++ b/Kernel/i386.h @@ -120,6 +120,16 @@ public: sti(); } + void temporarily_cli() + { + cli(); + } + + void temporarily_sti() + { + sti(); + } + private: dword m_flags; }; diff --git a/LibGUI/GTextBox.cpp b/LibGUI/GTextBox.cpp index fb761fcdf1..fd8adbae4f 100644 --- a/LibGUI/GTextBox.cpp +++ b/LibGUI/GTextBox.cpp @@ -46,7 +46,7 @@ void GTextBox::paint_event(GPaintEvent&) if (ch == ' ') continue; int x = inner_rect.x() + (i * font().glyph_width()); - painter.draw_bitmap({x, y}, font().glyph_bitmap(ch), Color::Black); + painter.draw_glyph({x, y}, ch, Color::Black); } if (is_focused() && m_cursor_blink_state) { diff --git a/SharedGraphics/Font.cpp b/SharedGraphics/Font.cpp index 54ddf5dff4..0a9c30474a 100644 --- a/SharedGraphics/Font.cpp +++ b/SharedGraphics/Font.cpp @@ -4,6 +4,8 @@ #include #ifdef KERNEL +#include +#include #include #include #endif @@ -34,6 +36,20 @@ static constexpr const char* error_glyph { static Font* s_default_font; static Font* s_default_bold_font; +struct FontFileHeader { + char magic[4]; + byte glyph_width; + byte glyph_height; + byte type; + byte unused[7]; + char name[64]; +} PACKED; + +static inline constexpr size_t font_file_size(unsigned glyph_height) +{ + return sizeof(FontFileHeader) + 256 * sizeof(dword) * glyph_height; +} + void Font::initialize() { s_default_font = nullptr; @@ -53,9 +69,10 @@ Font& Font::default_font() kprintf("Failed to open default font (%s)\n", default_font_path); ASSERT_NOT_REACHED(); } - auto buffer = descriptor->read_entire_file(*current); - ASSERT(buffer); - s_default_font = Font::load_from_memory(buffer.pointer()).leak_ref(); + auto* region = current->allocate_file_backed_region(LinearAddress(), font_file_size(10), descriptor->inode(), "default_font", /*readable*/true, /*writable*/false); + ASSERT(region); + region->page_in(); + s_default_font = Font::load_from_memory(region->laddr().as_ptr()).leak_ref(); #endif ASSERT(s_default_font); } @@ -72,12 +89,15 @@ Font& Font::default_bold_font() int error; auto descriptor = VFS::the().open(default_bold_font_path, error, 0, 0, *VFS::the().root_inode()); if (!descriptor) { - kprintf("Failed to open default font (%s)\n", default_bold_font_path); + kprintf("Failed to open default bold font (%s)\n", default_bold_font_path); ASSERT_NOT_REACHED(); } - auto buffer = descriptor->read_entire_file(*current); - ASSERT(buffer); - s_default_bold_font = Font::load_from_memory(buffer.pointer()).leak_ref(); + auto* region = current->allocate_file_backed_region(LinearAddress(), font_file_size(10), descriptor->inode(), "default_bold_font", /*readable*/true, /*writable*/false); + ASSERT(region); + ASSERT_INTERRUPTS_ENABLED(); + region->page_in(); + ASSERT_INTERRUPTS_ENABLED(); + s_default_bold_font = Font::load_from_memory(region->laddr().as_ptr()).leak_ref(); #endif ASSERT(s_default_bold_font); } @@ -86,54 +106,28 @@ Font& Font::default_bold_font() RetainPtr Font::clone() const { - size_t bytes_per_glyph = glyph_width() * glyph_height(); + size_t bytes_per_glyph = sizeof(dword) * glyph_height(); // FIXME: This is leaked! - char** new_glyphs = static_cast(kmalloc(sizeof(char*) * 256)); - for (unsigned i = 0; i < 256; ++i) { - new_glyphs[i] = static_cast(kmalloc(bytes_per_glyph)); - if (i >= m_first_glyph && i <= m_last_glyph) { - memcpy(new_glyphs[i], m_glyphs[i - m_first_glyph], bytes_per_glyph); - } else { - memset(new_glyphs[i], ' ', bytes_per_glyph); - } - } - return adopt(*new Font(m_name, new_glyphs, m_glyph_width, m_glyph_height, 0, 255)); + auto* new_rows = static_cast(kmalloc(bytes_per_glyph * 256)); + memcpy(new_rows, m_rows, bytes_per_glyph * 256); + return adopt(*new Font(m_name, new_rows, m_glyph_width, m_glyph_height)); } -Font::Font(const String& name, const char* const* glyphs, byte glyph_width, byte glyph_height, byte first_glyph, byte last_glyph) +Font::Font(const String& name, unsigned* rows, byte glyph_width, byte glyph_height) : m_name(name) - , m_glyphs(glyphs) + , m_rows(rows) , m_glyph_width(glyph_width) , m_glyph_height(glyph_height) - , m_first_glyph(first_glyph) - , m_last_glyph(last_glyph) { ASSERT(m_glyph_width == error_glyph_width); ASSERT(m_glyph_height == error_glyph_height); m_error_bitmap = CharacterBitmap::create_from_ascii(error_glyph, error_glyph_width, error_glyph_height); - for (unsigned ch = 0; ch < 256; ++ch) { - if (ch < m_first_glyph || ch > m_last_glyph) { - m_bitmaps[ch] = m_error_bitmap.copy_ref(); - continue; - } - const char* data = m_glyphs[(unsigned)ch - m_first_glyph]; - m_bitmaps[ch] = CharacterBitmap::create_from_ascii(data, m_glyph_width, m_glyph_height); - } } Font::~Font() { } -struct FontFileHeader { - char magic[4]; - byte glyph_width; - byte glyph_height; - byte type; - byte unused[7]; - char name[64]; -} PACKED; - RetainPtr Font::load_from_memory(const byte* data) { auto& header = *reinterpret_cast(data); @@ -146,25 +140,8 @@ RetainPtr Font::load_from_memory(const byte* data) return nullptr; } - auto* glyphs_ptr = reinterpret_cast(data + sizeof(FontFileHeader)); - - char** new_glyphs = static_cast(kmalloc(sizeof(char*) * 256)); - for (unsigned glyph_index = 0; glyph_index < 256; ++glyph_index) { - new_glyphs[glyph_index] = static_cast(kmalloc(header.glyph_width * header.glyph_height)); - char* bitptr = new_glyphs[glyph_index]; - for (unsigned y = 0; y < header.glyph_height; ++y) { - unsigned pattern = *(glyphs_ptr++); - for (unsigned x = 0; x < header.glyph_width; ++x) { - if (pattern & (1u << x)) { - *(bitptr++) = '#'; - } else { - *(bitptr++) = ' '; - } - } - } - } - - return adopt(*new Font(String(header.name), new_glyphs, header.glyph_width, header.glyph_height, 0, 255)); + auto* rows = (unsigned*)(data + sizeof(FontFileHeader)); + return adopt(*new Font(String(header.name), rows, header.glyph_width, header.glyph_height)); } #ifdef USERLAND @@ -185,10 +162,8 @@ RetainPtr Font::load_from_file(const String& path) } auto font = load_from_memory(mapped_file); - int rc = munmap(mapped_file, 4096 * 3); - ASSERT(rc == 0); - rc = close(fd); + int rc = close(fd); ASSERT(rc == 0); return font; } @@ -215,19 +190,7 @@ bool Font::write_to_file(const String& path) BufferStream stream(buffer); stream << ByteBuffer::wrap((byte*)&header, sizeof(FontFileHeader)); - - for (unsigned glyph_index = 0; glyph_index < 256; ++glyph_index) { - auto* glyph_bits = (byte*)m_glyphs[glyph_index]; - for (unsigned y = 0; y < m_glyph_height; ++y) { - unsigned pattern = 0; - for (unsigned x = 0; x < m_glyph_width; ++x) { - if (glyph_bits[y * m_glyph_width + x] == '#') { - pattern |= 1 << x; - } - } - stream << pattern; - } - } + stream << ByteBuffer::wrap((byte*)m_rows, (256 * bytes_per_glyph)); ASSERT(stream.at_end()); ssize_t nwritten = write(fd, buffer.pointer(), buffer.size()); diff --git a/SharedGraphics/Font.h b/SharedGraphics/Font.h index 0b560d822c..ebc64b6930 100644 --- a/SharedGraphics/Font.h +++ b/SharedGraphics/Font.h @@ -6,6 +6,38 @@ #include #include +// FIXME: Make a MutableGlyphBitmap buddy class for FontEditor instead? +class GlyphBitmap { + friend class Font; +public: + const unsigned* rows() const { return m_rows; } + unsigned row(unsigned index) const { return m_rows[index]; } + + bool bit_at(int x, int y) const { return row(y) & (1 << x); } + void set_bit_at(int x, int y, bool b) + { + auto& mutable_row = const_cast(m_rows)[y]; + if (b) + mutable_row |= 1 << x; + else + mutable_row &= ~(1 << x); + } + + Size size() const { return m_size; } + int width() const { return m_size.width(); } + int height() const { return m_size.height(); } + +private: + GlyphBitmap(const unsigned* rows, Size size) + : m_rows(rows) + , m_size(size) + { + } + + const unsigned* m_rows { nullptr }; + Size m_size; +}; + class Font : public Retainable { public: static Font& default_font(); @@ -22,7 +54,7 @@ public: ~Font(); - const CharacterBitmap& glyph_bitmap(char ch) const { return *m_bitmaps[(byte)ch]; } + GlyphBitmap glyph_bitmap(char ch) const { return GlyphBitmap(&m_rows[(byte)ch * m_glyph_height], { m_glyph_width, m_glyph_height }); } byte glyph_width() const { return m_glyph_width; } byte glyph_height() const { return m_glyph_height; } @@ -33,17 +65,14 @@ public: static void initialize(); private: - Font(const String& name, const char* const* glyphs, byte glyph_width, byte glyph_height, byte first_glyph, byte last_glyph); + Font(const String& name, unsigned* rows, byte glyph_width, byte glyph_height); String m_name; - const char* const* m_glyphs { nullptr }; - mutable RetainPtr m_bitmaps[256]; + unsigned* m_rows { nullptr }; + RetainPtr m_error_bitmap; byte m_glyph_width { 0 }; byte m_glyph_height { 0 }; - - byte m_first_glyph { 0 }; - byte m_last_glyph { 0 }; }; diff --git a/SharedGraphics/Painter.cpp b/SharedGraphics/Painter.cpp index 375ac36baa..a9965c713b 100644 --- a/SharedGraphics/Painter.cpp +++ b/SharedGraphics/Painter.cpp @@ -206,6 +206,29 @@ void Painter::draw_bitmap(const Point& p, const CharacterBitmap& bitmap, Color c } } +void Painter::draw_bitmap(const Point& p, const GlyphBitmap& bitmap, Color color) +{ + Rect rect { p, bitmap.size() }; + rect.move_by(m_translation); + auto clipped_rect = Rect::intersection(rect, m_clip_rect); + if (clipped_rect.is_empty()) + return; + const int first_row = clipped_rect.top() - rect.top(); + const int last_row = clipped_rect.bottom() - rect.top(); + const int first_column = clipped_rect.left() - rect.left(); + const int last_column = clipped_rect.right() - rect.left(); + RGBA32* dst = m_target->scanline(clipped_rect.y()) + clipped_rect.x(); + const size_t dst_skip = m_target->width(); + + for (int row = first_row; row <= last_row; ++row) { + for (int j = 0; j <= (last_column - first_column); ++j) { + if (bitmap.bit_at(j, row)) + dst[j] = color.value(); + } + dst += dst_skip; + } +} + FLATTEN void Painter::draw_glyph(const Point& point, char ch, Color color) { draw_bitmap(point, font().glyph_bitmap(ch), color); diff --git a/SharedGraphics/Painter.h b/SharedGraphics/Painter.h index 74435f23d1..ef0fed89da 100644 --- a/SharedGraphics/Painter.h +++ b/SharedGraphics/Painter.h @@ -7,6 +7,7 @@ #include class CharacterBitmap; +class GlyphBitmap; class GraphicsBitmap; class Font; @@ -26,6 +27,7 @@ public: void fill_rect_with_gradient(const Rect&, Color gradient_start, Color gradient_end); void draw_rect(const Rect&, Color); void draw_bitmap(const Point&, const CharacterBitmap&, Color = Color()); + void draw_bitmap(const Point&, const GlyphBitmap&, Color = Color()); void set_pixel(const Point&, Color); void draw_line(const Point&, const Point&, Color); void draw_focus_rect(const Rect&); -- cgit v1.2.3