Skip to content

remove some redundant variables and type params #971

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

Merged
merged 7 commits into from
Aug 12, 2019
Merged

remove some redundant variables and type params #971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fe08934
remove redundant variable init
SimonCropp Aug 12, 2019
3c05459
redundant variables
SimonCropp Aug 12, 2019
ab174fd
remove redundant tileY variable
SimonCropp Aug 12, 2019
21de86c
remove redundant sum variable
SimonCropp Aug 12, 2019
88d3ff8
redundant mcu variable
SimonCropp Aug 12, 2019
4e9bb60
redundant type params
SimonCropp Aug 12, 2019
215b041
Revert "remove redundant sum variable"
SimonCropp Aug 12, 2019
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
2 changes: 1 addition & 1 deletion src/ImageSharp/Advanced/AotCompilerTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private static void AotCompileDithering<TPixel>()
{
var test = new FloydSteinbergDiffuser();
TPixel pixel = default;
test.Dither<TPixel>(new ImageFrame<TPixel>(Configuration.Default, 1, 1), pixel, pixel, 0, 0, 0, 0, 0, 0);
test.Dither(new ImageFrame<TPixel>(Configuration.Default, 1, 1), pixel, pixel, 0, 0, 0, 0, 0, 0);
}

/// <summary>
Expand Down
10 changes: 0 additions & 10 deletions src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ private void ParseBaselineDataNonInterleaved()
dcHuffmanTable.Configure();
acHuffmanTable.Configure();

int mcu = 0;
for (int j = 0; j < h; j++)
{
Span<Block8x8> blockSpan = component.SpectralBlocks.GetRowSpan(j);
Expand All @@ -228,9 +227,6 @@ ref Unsafe.Add(ref blockRef, i),
ref dcHuffmanTable,
ref acHuffmanTable);

// Every data block is an MCU, so countdown the restart interval
mcu++;

this.HandleRestart();
}
}
Expand Down Expand Up @@ -379,7 +375,6 @@ private void ParseProgressiveDataNonInterleaved()
ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId];
dcHuffmanTable.Configure();

int mcu = 0;
for (int j = 0; j < h; j++)
{
Span<Block8x8> blockSpan = component.SpectralBlocks.GetRowSpan(j);
Expand All @@ -397,8 +392,6 @@ private void ParseProgressiveDataNonInterleaved()
ref Unsafe.Add(ref blockRef, i),
ref dcHuffmanTable);

// Every data block is an MCU, so countdown the restart interval
mcu++;
this.HandleRestart();
}
}
Expand All @@ -408,7 +401,6 @@ ref Unsafe.Add(ref blockRef, i),
ref HuffmanTable acHuffmanTable = ref this.acHuffmanTables[component.ACHuffmanTableId];
acHuffmanTable.Configure();

int mcu = 0;
for (int j = 0; j < h; j++)
{
Span<Block8x8> blockSpan = component.SpectralBlocks.GetRowSpan(j);
Expand All @@ -425,8 +417,6 @@ ref Unsafe.Add(ref blockRef, i),
ref Unsafe.Add(ref blockRef, i),
ref acHuffmanTable);

// Every data block is an MCU, so countdown the restart interval
mcu++;
this.HandleRestart();
}
}
Expand Down
15 changes: 3 additions & 12 deletions ...arp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,25 +77,22 @@ protected override void OnFrameApply(ImageFrame<TPixel> source, Rectangle source
new ParallelOptions { MaxDegreeOfParallelism = configuration.MaxDegreeOfParallelism },
index =>
{
int cdfX = 0;
int tileX = 0;
int tileY = 0;
int y = tileYStartPositions[index].y;
int cdfYY = tileYStartPositions[index].cdfY;

// It's unfortunate that we have to do this per iteration.
ref TPixel sourceBase = ref source.GetPixelReference(0, 0);

cdfX = 0;
int cdfX = 0;
for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth)
{
tileY = 0;
int tileY = 0;
int yEnd = Math.Min(y + tileHeight, sourceHeight);
int xEnd = Math.Min(x + tileWidth, sourceWidth);
for (int dy = y; dy < yEnd; dy++)
{
int dyOffSet = dy * sourceWidth;
tileX = 0;
int tileX = 0;
for (int dx = x; dx < xEnd; dx++)
{
ref TPixel pixel = ref Unsafe.Add(ref sourceBase, dyOffSet + dx);
Expand Down Expand Up @@ -221,7 +218,6 @@ private static void ProcessBorderColumn(
int xEnd,
int luminanceLevels)
{
int halfTileWidth = tileWidth / 2;
int halfTileHeight = tileHeight / 2;

int cdfY = 0;
Expand All @@ -232,13 +228,11 @@ private static void ProcessBorderColumn(
for (int dy = y; dy < yLimit; dy++)
{
int dyOffSet = dy * sourceWidth;
int tileX = halfTileWidth;
for (int dx = xStart; dx < xEnd; dx++)
{
ref TPixel pixel = ref Unsafe.Add(ref pixelBase, dyOffSet + dx);
float luminanceEqualized = InterpolateBetweenTwoTiles(pixel, cdfData, cdfX, cdfY, cdfX, cdfY + 1, tileY, tileHeight, luminanceLevels);
pixel.FromVector4(new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, pixel.ToVector4().W));
tileX++;
}

tileY++;
Expand Down Expand Up @@ -277,7 +271,6 @@ private static void ProcessBorderRow(
int cdfX = 0;
for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth)
{
int tileY = 0;
for (int dy = yStart; dy < yEnd; dy++)
{
int dyOffSet = dy * sourceWidth;
Expand All @@ -290,8 +283,6 @@ private static void ProcessBorderRow(
pixel.FromVector4(new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, pixel.ToVector4().W));
tileX++;
}

tileY++;
}

cdfX++;
Expand Down
2 changes: 1 addition & 1 deletion tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void NonMutatingEncodePreservesPaletteCount()
outStream.Position = 0;
var clone = Image.Load<Rgba32>(outStream);

GifMetadata cloneMetaData = clone.Metadata.GetFormatMetadata<GifMetadata>(GifFormat.Instance);
GifMetadata cloneMetaData = clone.Metadata.GetFormatMetadata(GifFormat.Instance);
Copy link
Member

@dlemstra dlemstra Aug 12, 2019

Choose a reason for hiding this comment

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

Maybe better to use var cloneMetaData here instead?

Copy link
Contributor Author

@SimonCropp SimonCropp Aug 12, 2019

Choose a reason for hiding this comment

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

do u want another pr that makes the use of var consistent?

Copy link
Member

@dlemstra dlemstra Aug 12, 2019

Choose a reason for hiding this comment

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

That would be nice, but you can probably sneak this one in?

Copy link
Member

@antonfirsov antonfirsov Aug 12, 2019

Choose a reason for hiding this comment

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

I think, we should merge it as-is now to keep the ball rolling.

Copy link
Contributor Author

@SimonCropp SimonCropp Aug 12, 2019

Choose a reason for hiding this comment

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

@dlemstra @antonfirsov @JimBobSquarePants so re var. whats your preferences? "just var everywhere"?

Copy link
Member

@JimBobSquarePants JimBobSquarePants Aug 12, 2019

Choose a reason for hiding this comment

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

The ide should hint via editorconfig. Var only when the type is apparent.

Copy link
Contributor Author

@SimonCropp SimonCropp Aug 12, 2019

Choose a reason for hiding this comment

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

weird. i dont get a hint?

Copy link
Member

@JimBobSquarePants JimBobSquarePants Aug 12, 2019

Choose a reason for hiding this comment

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

Very weird, what’s your dev setup? Any plugins etc?

Copy link
Contributor Author

@SimonCropp SimonCropp Aug 12, 2019

Choose a reason for hiding this comment

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

I will troubleshoot when i get home, and get back to you

Assert.Equal(metaData.ColorTableMode, cloneMetaData.ColorTableMode);

// Gifiddle and Cyotek GifInfo say this image has 64 colors.
Expand Down
10 changes: 5 additions & 5 deletions tests/ImageSharp.Tests/Image/ImageSaveTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void SavePath()
this.fileSystem.Setup(x => x.Create("path.png")).Returns(stream);
this.Image.Save("path.png");

this.encoder.Verify(x => x.Encode<Rgba32>(this.Image, stream));
this.encoder.Verify(x => x.Encode(this.Image, stream));
}


Expand All @@ -64,15 +64,15 @@ public void SavePathWithEncoder()

this.Image.Save("path.jpg", this.encoderNotInFormat.Object);

this.encoderNotInFormat.Verify(x => x.Encode<Rgba32>(this.Image, stream));
this.encoderNotInFormat.Verify(x => x.Encode(this.Image, stream));
}

[Fact]
public void ToBase64String()
{
string str = this.Image.ToBase64String(this.localImageFormat.Object);

this.encoder.Verify(x => x.Encode<Rgba32>(this.Image, It.IsAny<Stream>()));
this.encoder.Verify(x => x.Encode(this.Image, It.IsAny<Stream>()));
}

[Fact]
Expand All @@ -81,7 +81,7 @@ public void SaveStreamWithMime()
Stream stream = new MemoryStream();
this.Image.Save(stream, this.localImageFormat.Object);

this.encoder.Verify(x => x.Encode<Rgba32>(this.Image, stream));
this.encoder.Verify(x => x.Encode(this.Image, stream));
}

[Fact]
Expand All @@ -91,7 +91,7 @@ public void SaveStreamWithEncoder()

this.Image.Save(stream, this.encoderNotInFormat.Object);

this.encoderNotInFormat.Verify(x => x.Encode<Rgba32>(this.Image, stream));
this.encoderNotInFormat.Verify(x => x.Encode(this.Image, stream));
}

public void Dispose()
Expand Down