From 81c3ad3da5328350349216f249a5fe76ac974caf Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 30 Sep 2022 19:49:54 +0200 Subject: [PATCH] Revert "Fill read buffer in constructor and when IsInReadBuffer() is false" This reverts commit e066b839. --- src/ImageSharp/IO/BufferedReadStream.cs | 6 +-- .../IO/BufferedReadStreamTests.cs | 38 +++++-------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 39d294ae54..7e233c9655 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -49,6 +49,7 @@ internal sealed class BufferedReadStream : Stream this.BaseStream = stream; this.Length = stream.Length; + this.Position = (int)stream.Position; this.BufferSize = configuration.StreamProcessingBufferSize; this.maxBufferIndex = this.BufferSize - 1; this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); @@ -58,8 +59,8 @@ internal sealed class BufferedReadStream : Stream this.pinnedReadBuffer = (byte*)this.readBufferHandle.Pointer; } - this.Position = (int)stream.Position; - this.FillReadBuffer(); + // This triggers a full read on first attempt. + this.readBufferIndex = this.BufferSize; } /// @@ -98,7 +99,6 @@ internal sealed class BufferedReadStream : Stream this.BaseStream.Seek(value, SeekOrigin.Begin); this.readerPosition = value; this.readBufferIndex = this.BufferSize; - this.FillReadBuffer(); } } } diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index 619003a64d..6a9c344860 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System.IO; using SixLabors.ImageSharp.IO; namespace SixLabors.ImageSharp.Tests.IO; @@ -10,10 +9,13 @@ public class BufferedReadStreamTests { private readonly Configuration configuration; - public BufferedReadStreamTests() => this.configuration = Configuration.CreateDefaultInstance(); + public BufferedReadStreamTests() + { + this.configuration = Configuration.CreateDefaultInstance(); + } public static readonly TheoryData BufferSizes = - new() + new TheoryData() { 1, 2, 4, 8, 16, 97, 503, @@ -350,31 +352,6 @@ public class BufferedReadStreamTests } } - [Fact] - public void BufferedStreamSettingPositionWorksMultipleTimes() - { - // This test replicates an issue, which was caused by setting the stream position multiple times. - // This was then lead to the read buffer not being filled correctly. - int bufferSize = 16; - this.configuration.StreamProcessingBufferSize = bufferSize; - using MemoryStream stream = this.CreateTestStream(bufferSize * 2); - byte[] expected = stream.ToArray(); - using var reader = new BufferedReadStream(this.configuration, stream); - - // Read more then fits into the buffer. - for (int i = 0; i < 20; i++) - { - reader.ReadByte(); - } - - // Set the Position twice. - reader.Position = 10; - reader.Position = 3; - - int actual = reader.ReadByte(); - Assert.Equal(expected[3], actual); - } - private MemoryStream CreateTestStream(int length) { var buffer = new byte[length]; @@ -393,6 +370,9 @@ public class BufferedReadStreamTests { } - public override int Read(byte[] buffer, int offset, int count) => base.Read(buffer, offset, 1); + public override int Read(byte[] buffer, int offset, int count) + { + return base.Read(buffer, offset, 1); + } } }