Skip to content

Conversation

@brianpopow
Copy link
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Just a small improvement in writing 8 bit bitmaps with a palette: Converting the quantized colors to rgb is now done as bulk convert.


ReadOnlySpan<TPixel> quantizedColors = quantized.Palette.Span;
var color = default(Rgba32);
PixelOperations<TPixel>.Instance.ToRgba32(Configuration.Default, quantizedColors, rgbColors);
Copy link
Member

Choose a reason for hiding this comment

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

This should be the configuration that the memory allocator is derived from.

Comment on lines 342 to 349
using IMemoryOwner<Rgba32> rgbColorsBuffer = this.memoryAllocator.Allocate<Rgba32>(quantized.Palette.Length);
Span<Rgba32> rgbColors = rgbColorsBuffer.GetSpan();

ReadOnlySpan<TPixel> quantizedColors = quantized.Palette.Span;
var color = default(Rgba32);
PixelOperations<TPixel>.Instance.ToRgba32(Configuration.Default, quantizedColors, rgbColors);

// TODO: Use bulk conversion here for better perf
int idx = 0;
foreach (TPixel quantizedColor in quantizedColors)
foreach (Rgba32 color in rgbColors)
Copy link
Member

Choose a reason for hiding this comment

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

Why not something like:

PixelOperations<TPixel>.Instance.ToRgba32(Configuration.Default, quantizedColors, MemoryMarshal.Cast<byte, Rgba32>(colorPalette));
ref Span<uint> colorPaletteAsUInt= MemoryMarshal.Cast<byte, uint>(colorPalette);
for (int i =0;i<colorPaletteAsUInt;i++)
  colorPaletteAsUInt[i] = colorPaletteAsUInt[i] & 0xFFFFFF00;

Copy link
Member

Choose a reason for hiding this comment

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

Tidy!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah that will do, it just needs to be ToBgra instead

Copy link
Member

Choose a reason for hiding this comment

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

Which should be fast enough with the new shuffle intrinsics

@brianpopow brianpopow changed the title Use bulk conversion to rgba in Write8BitColor Use bulk conversion in Write8BitColor Nov 15, 2020
@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #1428 (04881fd) into master (59b65a9) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1428      +/-   ##
==========================================
- Coverage   83.08%   83.08%   -0.01%     
==========================================
  Files         707      707              
  Lines       31839    31834       -5     
  Branches     3590     3590              
==========================================
- Hits        26454    26449       -5     
  Misses       4668     4668              
  Partials      717      717              
Flag Coverage Δ
unittests 83.08% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs 96.50% <100.00%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59b65a9...04881fd. Read the comment docs.

@JimBobSquarePants JimBobSquarePants merged commit dea0737 into master Nov 16, 2020
@JimBobSquarePants JimBobSquarePants deleted the bp/bmp8BitBulkConv branch November 16, 2020 11:41
JimBobSquarePants added a commit that referenced this pull request Mar 13, 2021
Use bulk conversion in Write8BitColor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants