gnash-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Gnash-commit] /srv/bzr/gnash/trunk r10443: Reduce duplication in Define


From: Benjamin Wolsey
Subject: [Gnash-commit] /srv/bzr/gnash/trunk r10443: Reduce duplication in DefineBits parsing.
Date: Tue, 16 Dec 2008 11:17:07 +0100
User-agent: Bazaar (1.5)

------------------------------------------------------------
revno: 10443
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Tue 2008-12-16 11:17:07 +0100
message:
  Reduce duplication in DefineBits parsing.
  
  Adjust existing RGB values when merging alpha to jpeg data. Fixes
  bugs #23131, #21818, part of bug #20257, and the bad rendering
  in bug #24201.
modified:
  backend/render_handler_agg_style.h
  libbase/GnashImage.cpp
  libcore/parser/bitmap_character_def.h
  libcore/parser/morph2_character_def.cpp
  libcore/swf/tag_loaders.cpp
  testsuite/DummyMovieDefinition.h
    ------------------------------------------------------------
    revno: 10442.1.1
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 07:45:58 +0100
    message:
      Minor header and declaration cleanups.
    modified:
      libcore/parser/bitmap_character_def.h
      libcore/parser/morph2_character_def.cpp
      libcore/swf/tag_loaders.cpp
      testsuite/DummyMovieDefinition.h
    ------------------------------------------------------------
    revno: 10442.1.2
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 08:01:57 +0100
    message:
      Start cleanup of define bits tag parsing.
    modified:
      libcore/swf/tag_loaders.cpp
    ------------------------------------------------------------
    revno: 10442.1.3
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 08:51:48 +0100
    message:
      Progress.
    modified:
      libcore/swf/tag_loaders.cpp
    ------------------------------------------------------------
    revno: 10442.1.4
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 09:45:55 +0100
    message:
      More code reduction.
    modified:
      libbase/GnashImage.cpp
      libcore/swf/tag_loaders.cpp
    ------------------------------------------------------------
    revno: 10442.1.5
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 09:58:40 +0100
    message:
      Premultiply alpha when adding to jpeg images, as all other bitmaps are
      expected to have premultiplied RGB. Fixes the bad alpha display.
    modified:
      libbase/GnashImage.cpp
      libcore/swf/tag_loaders.cpp
    ------------------------------------------------------------
    revno: 10442.1.6
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 10:24:27 +0100
    message:
      Complete removal of code duplication. DEFINEBITSLOSSLESS2 (with alpha) and
      pixel format 4 seemed wrong, as alpha was stored to the first byte of
      every pixel. Gnash expects RGBA. This is now changed, but I have no
      examples of such a tag in any SWF for testing.
    modified:
      libcore/swf/tag_loaders.cpp
    ------------------------------------------------------------
    revno: 10442.1.7
    committer: Benjamin Wolsey <address@hidden>
    branch nick: work
    timestamp: Tue 2008-12-16 10:58:40 +0100
    message:
      Use the std::min of alpha and the respective pixel value, which makes 
lumi.swf
      display correctly.
    modified:
      backend/render_handler_agg_style.h
      libbase/GnashImage.cpp
      libcore/swf/tag_loaders.cpp
=== modified file 'backend/render_handler_agg_style.h'
--- a/backend/render_handler_agg_style.h        2008-10-19 18:04:05 +0000
+++ b/backend/render_handler_agg_style.h        2008-12-16 09:58:40 +0000
@@ -110,13 +110,10 @@
 class agg_style_bitmap : public agg_style_base
 {
 public:
-
-  bool m_force_premultiply;
     
   agg_style_bitmap(int width, int height, int rowlen, boost::uint8_t* data, 
     gnash::SWFMatrix mat, gnash::cxform cx) :
     
-    m_force_premultiply(false),
     m_rbuf(data, width, height, rowlen),  
     m_pixf(m_rbuf),
     m_img_src(m_pixf),
@@ -154,13 +151,7 @@
         span->premultiply();
         ++span;
       }
-    } else 
-    if (m_force_premultiply) {
-      for (unsigned int i=0; i<len; i++) { 
-        span->premultiply();
-        span++;
-      }    
-    }
+    }  
   }
     
   

=== modified file 'libbase/GnashImage.cpp'
--- a/libbase/GnashImage.cpp    2008-11-27 08:19:11 +0000
+++ b/libbase/GnashImage.cpp    2008-12-16 09:58:40 +0000
@@ -158,12 +158,17 @@
     // Point to the first alpha byte
     boost::uint8_t* p = data();
 
-    // Set each 4th byte to the correct alpha value.
-    for (size_t i = 0; i < bufferLength; ++i) {
-        p += 3;
+    // Set each 4th byte to the correct alpha value and adjust the
+    // other values.
+    for (size_t i = 0; i < bufferLength; ++i, ++alphaData) {
+        *p = std::min(*p, *alphaData);
+        ++p;
+        *p = std::min(*p, *alphaData);
+        ++p;
+        *p = std::min(*p, *alphaData);
+        ++p;
         *p = *alphaData;
         ++p;
-        ++alphaData;
     }
 }
 

=== modified file 'libcore/parser/bitmap_character_def.h'
--- a/libcore/parser/bitmap_character_def.h     2008-10-27 16:05:13 +0000
+++ b/libcore/parser/bitmap_character_def.h     2008-12-16 06:45:58 +0000
@@ -53,7 +53,7 @@
 /// availability of a render_handler in order to transform
 /// ImageRGB or image::ImageRGBA to a bitmap_info.
 ///
-class bitmap_character_def : public ref_counted // @@ why not character_def ?
+class bitmap_character_def : public ref_counted 
 {
 
 public:

=== modified file 'libcore/parser/morph2_character_def.cpp'
--- a/libcore/parser/morph2_character_def.cpp   2008-11-07 08:37:30 +0000
+++ b/libcore/parser/morph2_character_def.cpp   2008-12-16 06:45:58 +0000
@@ -26,7 +26,6 @@
 #include "SWFStream.h"
 #include "render.h"
 #include "movie_definition.h"
-#include "bitmap_character_def.h"
 #include "MovieClip.h"
 
 

=== modified file 'libcore/swf/tag_loaders.cpp'
--- a/libcore/swf/tag_loaders.cpp       2008-12-01 19:24:05 +0000
+++ b/libcore/swf/tag_loaders.cpp       2008-12-16 09:58:40 +0000
@@ -519,247 +519,179 @@
 
     in.ensureBytes(2+2+2+1); // the initial header 
 
-    boost::uint16_t    character_id = in.read_u16();
-    boost::uint8_t    bitmap_format = in.read_u8();    // 3 == 8 bit, 4 == 16 
bit, 5 == 32 bit
-    boost::uint16_t    width = in.read_u16();
-    boost::uint16_t    height = in.read_u16();
-
-    IF_VERBOSE_PARSE
-    (
-    log_parse(_("  defbitslossless2: tag = %d, id = %d, "
+    boost::uint16_t character_id = in.read_u16();
+
+    // 3 == 8 bit, 4 == 16 bit, 5 == 32 bit
+    boost::uint8_t bitmap_format = in.read_u8();
+    boost::uint16_t width = in.read_u16();
+    boost::uint16_t height = in.read_u16();
+
+    IF_VERBOSE_PARSE(
+        log_parse(_("  defbitslossless2: tag = %d, id = %d, "
             "fmt = %d, w = %d, h = %d"),
-          tag, character_id, bitmap_format, width, height);
+            tag, character_id, bitmap_format, width, height);
     );
 
-    if (width == 0 || height == 0)
-    {
+    if (!width || !height) {
          IF_VERBOSE_MALFORMED_SWF(
-            log_swferror(_("Bitmap character %d has a height or width of 0."), 
character_id);
+            log_swferror(_("Bitmap character %d has a height or width of 0"),
+                character_id);
         );   
         return;  
     }
 
-    // TODO: there's a lot of duplicated code in this function, we should 
clean it up
+    // No need to parse any further if it already exists, as we aren't going
+    // to add it.
+    if ( m.get_bitmap_character_def(character_id) )
+    {
+        IF_VERBOSE_MALFORMED_SWF(
+            log_swferror(_("DEFINEBITSLOSSLESS: Duplicate id (%d) "
+                           "for bitmap character - discarding it"),
+                character_id);
+        );
+    }
 
-    //bitmap_info*    bi = NULL;
 #ifndef HAVE_ZLIB_H
     log_error(_("gnash is not linked to zlib -- can't load zipped image 
data"));
     return;
 #else
-    if (tag == SWF::DEFINELOSSLESS) // 20
-    {
-
-        // RGB image data.
-        std::auto_ptr<GnashImage> image (new ImageRGB(width, height));
-
-        if (bitmap_format == 3)
-        {
-            // 8-bit data, preceded by a palette.
-
-            const int bytes_per_pixel = 1;
-
-                in.ensureBytes(1); // color table size
-            int color_table_size = in.read_u8();
-            color_table_size++;    // !! SWF stores one less than the actual 
size
-
-            int pitch = (width * bytes_per_pixel + 3) & ~3;
-
-            int buffer_bytes = color_table_size * 3 + pitch * height;
-            boost::scoped_array<boost::uint8_t> buffer ( new 
boost::uint8_t[buffer_bytes] );
-
-            inflate_wrapper(in, buffer.get(), buffer_bytes);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            boost::uint8_t* color_table = buffer.get();
-
-            for (int j = 0; j < height; j++)
-            {
-                boost::uint8_t*    image_in_row = buffer.get() + 
color_table_size * 3 + j * pitch;
-                boost::uint8_t*    image_out_row = image->scanline(j);
-                for (int i = 0; i < width; i++)
-                {
-                boost::uint8_t    pixel = image_in_row[i * bytes_per_pixel];
-                image_out_row[i * 3 + 0] = color_table[pixel * 3 + 0];
-                image_out_row[i * 3 + 1] = color_table[pixel * 3 + 1];
-                image_out_row[i * 3 + 2] = color_table[pixel * 3 + 2];
-                }
-            }
-
-        }
-        else if (bitmap_format == 4)
-        {
-            // 16 bits / pixel
-            const int bytes_per_pixel = 2;
-            int pitch = (width * bytes_per_pixel + 3) & ~3;
-
-            int buffer_bytes = pitch * height;
-            boost::scoped_array<boost::uint8_t> buffer ( new 
boost::uint8_t[buffer_bytes] );
-
-            inflate_wrapper(in, buffer.get(), buffer_bytes);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            for (int j = 0; j < height; j++)
-            {
-                boost::uint8_t*    image_in_row = buffer.get() + j * pitch;
-                boost::uint8_t*    image_out_row = image->scanline(j);
-                for (int i = 0; i < width; i++)
-                {
-                boost::uint16_t    pixel = image_in_row[i * 2] | 
(image_in_row[i * 2 + 1] << 8);
-
-                // @@ How is the data packed???  I'm just guessing here that 
it's 565!
-                image_out_row[i * 3 + 0] = (pixel >> 8) & 0xF8;    // red
-                image_out_row[i * 3 + 1] = (pixel >> 3) & 0xFC;    // green
-                image_out_row[i * 3 + 2] = (pixel << 3) & 0xF8;    // blue
-                }
-            }
-
-        }
-        else if (bitmap_format == 5)
-        {
-            // 32 bits / pixel, input is ARGB format (???)
-            const int bytes_per_pixel = 4;
-            int pitch = width * bytes_per_pixel;
-
-            int buffer_bytes = pitch * height;
-            boost::scoped_array<boost::uint8_t> buffer ( new 
boost::uint8_t[buffer_bytes] );
-
-            inflate_wrapper(in, buffer.get(), buffer_bytes);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            // Need to re-arrange ARGB into RGB.
-            for (int j = 0; j < height; j++)
-            {
-                boost::uint8_t*    image_in_row = buffer.get() + j * pitch;
-                boost::uint8_t*    image_out_row = image->scanline(j);
-                for (int i = 0; i < width; i++)
-                {
-                boost::uint8_t a = image_in_row[i * 4 + 0];
-                boost::uint8_t r = image_in_row[i * 4 + 1];
-                boost::uint8_t g = image_in_row[i * 4 + 2];
-                boost::uint8_t b = image_in_row[i * 4 + 3];
-                image_out_row[i * 3 + 0] = r;
-                image_out_row[i * 3 + 1] = g;
-                image_out_row[i * 3 + 2] = b;
-                a = a;    // Inhibit warning.
-                }
-            }
-
-        }
-
-        if ( m.get_bitmap_character_def(character_id) )
-        {
-            IF_VERBOSE_MALFORMED_SWF(
-                log_swferror(_("DEFINEBITSLOSSLESS: Duplicate id (%d) "
-                               "for bitmap character - discarding it"), 
character_id);
-            );
-        }
-        else
-        {
-            boost::intrusive_ptr<bitmap_character_def> ch = new 
bitmap_character_def(image);
-
-            // add image to movie, under character id.
-            m.add_bitmap_character_def(character_id, ch.get());
-        }
-    }
-    else
-    {
-        // RGBA image data.
-        assert(tag == SWF::DEFINELOSSLESS2); // 36
-
-        std::auto_ptr<GnashImage> image(new ImageRGBA(width, height));
-
-        if (bitmap_format == 3)
-        {
-            // 8-bit data, preceded by a palette.
-
-            const int bytes_per_pixel = 1;
-                in.ensureBytes(1); // color table size
-            int color_table_size = in.read_u8();
-            color_table_size++;    // !! SWF stores one less than the actual 
size
-
-            int pitch = (width * bytes_per_pixel + 3) & ~3;
-
-            int buffer_bytes = color_table_size * 4 + pitch * height;
-            boost::scoped_array<boost::uint8_t> buffer ( new 
boost::uint8_t[buffer_bytes] );
-
-            inflate_wrapper(in, buffer.get(), buffer_bytes);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            boost::uint8_t* color_table = buffer.get();
-
-        for (int j = 0; j < height; j++)
-        {
-            boost::uint8_t*    image_in_row = buffer.get() + color_table_size 
* 4 + j * pitch;
-            boost::uint8_t*    image_out_row = image->scanline(j);
-            for (int i = 0; i < width; i++)
-            {
-                boost::uint8_t    pixel = image_in_row[i * bytes_per_pixel];
-                image_out_row[i * 4 + 0] = color_table[pixel * 4 + 0];
-                image_out_row[i * 4 + 1] = color_table[pixel * 4 + 1];
-                image_out_row[i * 4 + 2] = color_table[pixel * 4 + 2];
-                image_out_row[i * 4 + 3] = color_table[pixel * 4 + 3];
-            }
-        }
-
-        }
-        else if (bitmap_format == 4)
-        {
-            // 16 bits / pixel
-            const int bytes_per_pixel = 2;
-            int pitch = (width * bytes_per_pixel + 3) & ~3;
-
-            int buffer_bytes = pitch * height;
-            boost::scoped_array<boost::uint8_t> buffer ( new 
boost::uint8_t[buffer_bytes] );
-
-            inflate_wrapper(in, buffer.get(), buffer_bytes);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            for (int j = 0; j < height; j++)
-            {
-                boost::uint8_t*    image_in_row = buffer.get() + j * pitch;
-                boost::uint8_t*    image_out_row = image->scanline(j);
-                for (int i = 0; i < width; i++)
-                {
-                    boost::uint16_t    pixel = image_in_row[i * 2] | 
(image_in_row[i * 2 + 1] << 8);
-
-                    // @@ How is the data packed???  I'm just guessing here 
that it's 565!
-                    image_out_row[i * 4 + 0] = 255;            // alpha
-                    image_out_row[i * 4 + 1] = (pixel >> 8) & 0xF8;    // red
-                    image_out_row[i * 4 + 2] = (pixel >> 3) & 0xFC;    // green
-                    image_out_row[i * 4 + 3] = (pixel << 3) & 0xF8;    // blue
-                }
-            }
-        }
-        else if (bitmap_format == 5)
-        {
-            // 32 bits / pixel, input is ARGB format
-
-            inflate_wrapper(in, image->data(), width * height * 4);
-            assert(in.tell() <= in.get_tag_end_position());
-
-            // Need to re-arrange ARGB into RGBA.
-            for (int j = 0; j < height; j++)
-            {
-                boost::uint8_t*    image_row = image->scanline(j);
-                for (int i = 0; i < width; i++)
-                {
-                    boost::uint8_t    a = image_row[i * 4 + 0];
-                    boost::uint8_t    r = image_row[i * 4 + 1];
-                    boost::uint8_t    g = image_row[i * 4 + 2];
-                    boost::uint8_t    b = image_row[i * 4 + 3];
-                    image_row[i * 4 + 0] = r;
-                    image_row[i * 4 + 1] = g;
-                    image_row[i * 4 + 2] = b;
-                    image_row[i * 4 + 3] = a;
-                }
-            }
-        }
-
-        boost::intrusive_ptr<bitmap_character_def> ch = new 
bitmap_character_def(image);
-
-        // add image to movie, under character id.
-        m.add_bitmap_character_def(character_id, ch.get());
-    }
+
+    unsigned short channels;
+    std::auto_ptr<GnashImage> image;
+    bool alpha = false;
+
+    switch (tag)
+    {
+        case SWF::DEFINELOSSLESS:
+            image.reset(new ImageRGB(width, height));
+            channels = 3;
+            break;
+        case SWF::DEFINELOSSLESS2:
+            image.reset(new ImageRGBA(width, height));
+            channels = 4;
+            alpha = true;
+            break;
+        default:
+            // This is already asserted.
+            std::abort();
+    }
+
+    unsigned short bytes_per_pixel;
+    int colorTableSize = 0;
+
+    switch (bitmap_format)
+    {
+        case 3:
+            bytes_per_pixel = 1;
+            in.ensureBytes(1);
+            // SWF stores one less than the actual size.
+            colorTableSize = in.read_u8() + 1;
+            break;
+
+        case 4:
+            bytes_per_pixel = 2;
+            break;
+
+        case 5:
+            bytes_per_pixel = 4;
+            break;
+
+        default:
+            log_error(_("Unknown bitmap format. Ignoring"));
+            return;
+    }
+
+    const size_t pitch = (width * bytes_per_pixel + 3) &~ 3;
+    const size_t bufSize = colorTableSize * channels + pitch * height;
+    boost::scoped_array<boost::uint8_t> buffer(new boost::uint8_t[bufSize]);
+
+    inflate_wrapper(in, buffer.get(), bufSize);
+    assert(in.tell() <= in.get_tag_end_position());
+
+    switch (bitmap_format)
+    {
+
+        case 3:
+        {
+            // 8-bit data, preceded by a palette.
+            boost::uint8_t* color_table = buffer.get();
+
+            for (int j = 0; j < height; j++)
+            {
+                boost::uint8_t* image_in_row = buffer.get() + 
+                    colorTableSize * channels + j * pitch;
+
+                boost::uint8_t*    image_out_row = image->scanline(j);
+                for (int i = 0; i < width; i++)
+                {
+                    boost::uint8_t pixel = image_in_row[i * bytes_per_pixel];
+                    image_out_row[i * channels + 0] = color_table[pixel * 
channels + 0];
+                    image_out_row[i * channels + 1] = color_table[pixel * 
channels + 1];
+                    image_out_row[i * channels + 2] = color_table[pixel * 
channels + 2];
+                    if (alpha) {
+                        image_out_row[i * channels + 3] = color_table[pixel * 
channels + 3];
+                    }
+                }
+            }
+            break;
+        }
+
+        case 4:
+            // 16 bits / pixel
+
+            for (int j = 0; j < height; j++)
+            {
+                boost::uint8_t* image_in_row = buffer.get() + j * pitch;
+                boost::uint8_t* image_out_row = image->scanline(j);
+                for (int i = 0; i < width; i++)
+                {
+                    boost::uint16_t pixel = image_in_row[i * 2] |
+                        (image_in_row[i * 2 + 1] << 8);
+
+                    // How is the data packed??? Whoever wrote this was just 
guessing
+                    // here that it's 565!
+                    image_out_row[i * channels + 0] = (pixel >> 8) & 0xF8;    
// red
+                    image_out_row[i * channels + 1] = (pixel >> 3) & 0xFC;    
// green
+                    image_out_row[i * channels + 2] = (pixel << 3) & 0xF8;    
// blue
+ 
+                    // This was saved to the first byte before, but that can 
hardly be correct.
+                    // Real examples of this format are rare to non-existent.
+                    if (alpha) {
+                        image_out_row[i * channels + 3] = 255;
+                    }
+                }
+            }
+            break;
+
+        case 5:
+            // Need to re-arrange ARGB into RGB or RGBA.
+            for (int j = 0; j < height; j++)
+            {
+                boost::uint8_t* inRow = buffer.get() + j * pitch;
+                boost::uint8_t* outRow = image->scanline(j);
+                const int inChannels = 4;
+
+                for (int i = 0; i < width; ++i)
+                {
+                    // Copy pixels 1-3.
+                    std::copy(&inRow[i * inChannels + 1], &inRow[i * 
inChannels + 4],
+                            &outRow[i * channels]);
+                    // Add the alpha channel if necessary.
+                    if (alpha) {
+                        outRow[i * channels + 3] = inRow[i * 4];
+                    }
+                }
+            }
+            break;
+
+    }
+
+
+    boost::intrusive_ptr<bitmap_character_def> ch =
+        new bitmap_character_def(image);
+
+    // add image to movie, under character id.
+    m.add_bitmap_character_def(character_id, ch.get());
 #endif // HAVE_ZLIB_H
 
 }

=== modified file 'testsuite/DummyMovieDefinition.h'
--- a/testsuite/DummyMovieDefinition.h  2008-01-21 23:26:48 +0000
+++ b/testsuite/DummyMovieDefinition.h  2008-12-16 06:45:58 +0000
@@ -25,10 +25,6 @@
 #include <string>
 #include <memory> // for auto_ptr
 
-// Forward declarations
-namespace gnash {
-       class bitmap_character_def;
-}
 
 namespace gnash
 {


reply via email to

[Prev in Thread] Current Thread [Next in Thread]